FEAT: changed the warning when targeting a label in an aggregated array using the group that created it to an error.

Author: gdementen

doc/source/changes/version_0_35.rst.inc

@@ -27,6 +27,13 @@ Backward incompatible changes
   :py:obj:`CheckedArray` now requires installing pydantic >= 2
   (closes :issue:`1075`).
 
+* the warning introduced in LArray 0.34 (see :issue:`994`) when targeting a
+  label in an aggregated array using the group that created it, has been
+  changed to an error. The aggregated array label should be used instead.
+  This was a seldom used feature which was complex to keep working and had a
+  significant performance cost in some cases, even when the feature is not
+  used.
+
 
 Future breaking changes
 ^^^^^^^^^^^^^^^^^^^^^^^

larray/core/axis.py

@@ -111,6 +111,23 @@ def _retarget_warn_msg(key, real_axis, current_eval=None, future_eval=None):
     )
 
 
+_GROUP_AS_AGGREGATED_LABEL_MSG_TEMPLATE = \
+    "Using a Group object which was used to create an aggregate to " \
+    "target its aggregated label is deprecated. " \
+    "Please use the aggregated label directly instead. " \
+    "In this case, you should use {potential_tick!r} instead of " \
+    "using {key!r}."
+
+
+def _group_as_aggregated_label_msg(key, potential_tick=None):
+    if potential_tick is None:
+        potential_tick = _to_tick(key)
+    return _GROUP_AS_AGGREGATED_LABEL_MSG_TEMPLATE.format(
+        potential_tick=potential_tick,
+        key=key
+    )
+
+
 class Axis(ABCAxis):
     r"""
     Represents an axis. It consists of a name and a list of labels.
@@ -933,16 +950,7 @@ def isscalar(k):
             if (not isinstance(self, AxisReference)
                     and key.axis.name == self.name
                     and key.name in self):
-                msg = (
-                    "Using a Group object which was used to create an "
-                    "aggregate to target its aggregated label is deprecated. "
-                    "Please use the aggregated label directly instead. "
-                    f"In this case, you should use {key.name!r} instead of "
-                    f"using {key!r}."
-                )
-                # let us hope the stacklevel does not vary by codepath
-                warnings.warn(msg, FutureWarning, stacklevel=7)
-                return LGroup(key.name, None, self)
+                raise ValueError(_group_as_aggregated_label_msg(key, key.name))
             # retarget a Group from another axis to this axis
             # TODO: uncomment this code once we actually retarget groups from
             #       other axes in LGroup.__init__
@@ -1027,14 +1035,12 @@ def index(self, key) -> Union[int, np.ndarray, slice]:
                         try:
                             res_idx = mapping[potential_tick]
                             if potential_tick != key.key:
-                                # only warn if no KeyError was raised (potential_tick is in mapping)
-                                msg = "Using a Group object which was used to create an aggregate to " \
-                                      "target its aggregated label is deprecated. " \
-                                      "Please use the aggregated label directly instead. " \
-                                      f"In this case, you should use {potential_tick!r} instead of " \
-                                      f"using {key!r}."
-                                # let us hope the stacklevel does not vary by codepath
-                                warnings.warn(msg, FutureWarning, stacklevel=8)
+                                raise ValueError(
+                                    _group_as_aggregated_label_msg(
+                                        key,
+                                        potential_tick
+                                    )
+                                )
                             return res_idx
                         except KeyError:
                             pass

larray/tests/test_array.py

@@ -20,7 +20,7 @@
                     from_lists, from_string, from_frame, from_series,
                     zip_array_values, zip_array_items, nan_to_num)
 from larray.core.axis import (
-    _to_ticks, _to_tick, _to_key, _retarget_warn_msg
+    _to_ticks, _to_key, _retarget_warn_msg, _group_as_aggregated_label_msg
 )
 from larray.util.misc import LHDFStore
 
@@ -33,16 +33,6 @@
 # E241: multiple spaces after ','
 
 
-GROUP_AS_AGGREGATED_LABEL_MSG_TEMPLATE = "Using a Group object which was used to create an aggregate to " \
-      "target its aggregated label is deprecated. " \
-      "Please use the aggregated label directly instead. " \
-      "In this case, you should use {potential_tick!r} instead of " \
-      "using {key!r}."
-
-def group_as_aggregated_label_msg(key):
-    return GROUP_AS_AGGREGATED_LABEL_MSG_TEMPLATE.format(potential_tick=_to_tick(key), key=key)
-
-
 # ================== #
 # Test Value Strings #
 # ================== #
@@ -2471,26 +2461,32 @@ def test_getitem_on_group_agg(array):
     # using an anonymous LGroup
     lg1 = b[b_group1]
     agg_arr = array.sum(a, c).sum(b=(lg1, b_group2, b_group2, all_b))
-    with must_warn(FutureWarning, msg=group_as_aggregated_label_msg(lg1), num_expected=5):
-        # the following should all be equivalent
-        assert agg_arr[lg1].shape == (6,)
-        assert agg_arr[(lg1,)].shape == (6,)
-        # these last three are only syntactic sugar differences
-        # (__getitem__ receives the *exact* same key)
-        assert agg_arr[(lg1, slice(None))].shape == (6,)
-        assert agg_arr[lg1, slice(None)].shape == (6,)
-        assert agg_arr[lg1, :].shape == (6,)
+    msg = _group_as_aggregated_label_msg(lg1)
+    # the following should all be equivalent
+    with must_raise(ValueError, msg=msg):
+        agg_arr[lg1]
+    with must_raise(ValueError, msg=msg):
+        agg_arr[(lg1,)]
+    with must_raise(ValueError, msg=msg):
+        agg_arr[(lg1, slice(None))]
+        # only syntactic sugar differences (__getitem__ gets *exact* same key)
+        # agg_arr[lg1, slice(None)]
+        # agg_arr[lg1, :]
 
     # using a named LGroup
     lg1 = b[b_group1] >> 'g1'
     agg_arr = array.sum(a, c).sum(b=(lg1, b_group2, b_group2, all_b))
-    with must_warn(FutureWarning, msg=group_as_aggregated_label_msg(lg1), num_expected=5):
-        # the following are all equivalent
-        assert agg_arr[lg1].shape == (6,)
-        assert agg_arr[(lg1,)].shape == (6,)
-        assert agg_arr[(lg1, slice(None))].shape == (6,)
-        assert agg_arr[lg1, slice(None)].shape == (6,)
-        assert agg_arr[lg1, :].shape == (6,)
+    msg = _group_as_aggregated_label_msg(lg1)
+    # the following should all be equivalent
+    with must_raise(ValueError, msg=msg):
+        agg_arr[lg1]
+    with must_raise(ValueError, msg=msg):
+        agg_arr[(lg1,)]
+    with must_raise(ValueError, msg=msg):
+        agg_arr[(lg1, slice(None))]
+        # only syntactic sugar differences (__getitem__ gets *exact* same key)
+        # agg_arr[lg1, slice(None)]
+        # agg_arr[lg1, :]
 
 
 def test_getitem_on_group_agg_nokw(array):
@@ -2512,24 +2508,32 @@ def test_getitem_on_group_agg_nokw(array):
     # using an anonymous LGroup
     lg1 = b[b_group1]
     agg_arr = array.sum(a, c).sum((lg1, b_group2, b_group3, all_b))
-    with must_warn(FutureWarning, msg=group_as_aggregated_label_msg(lg1), num_expected=5):
-        # the following are all equivalent
-        assert agg_arr[lg1].shape == (6,)
-        assert agg_arr[(lg1,)].shape == (6,)
-        assert agg_arr[(lg1, slice(None))].shape == (6,)
-        assert agg_arr[lg1, slice(None)].shape == (6,)
-        assert agg_arr[lg1, :].shape == (6,)
+    # the following are all equivalent
+    msg = _group_as_aggregated_label_msg(lg1)
+    with must_raise(ValueError, msg=msg):
+        agg_arr[lg1]
+    with must_raise(ValueError, msg=msg):
+        agg_arr[(lg1,)]
+    with must_raise(ValueError, msg=msg):
+        agg_arr[(lg1, slice(None))]
+        # only syntactic sugar differences (__getitem__ gets *exact* same key)
+        # agg_arr[lg1, slice(None)]
+        # agg_arr[lg1, :]
 
     # using a named LGroup
     lg1 = b[b_group1] >> 'g1'
     agg_arr = array.sum(a, c).sum((lg1, b_group2, b_group3, all_b))
-    with must_warn(FutureWarning, msg=group_as_aggregated_label_msg(lg1), num_expected=5):
-        # the following are all equivalent
-        assert agg_arr[lg1].shape == (6,)
-        assert agg_arr[(lg1,)].shape == (6,)
-        assert agg_arr[(lg1, slice(None))].shape == (6,)
-        assert agg_arr[lg1, slice(None)].shape == (6,)
-        assert agg_arr[lg1, :].shape == (6,)
+    msg = _group_as_aggregated_label_msg(lg1)
+    # the following are all equivalent
+    with must_raise(ValueError, msg=msg):
+        agg_arr[lg1]
+    with must_raise(ValueError, msg=msg):
+        agg_arr[(lg1,)]
+    with must_raise(ValueError, msg=msg):
+        agg_arr[(lg1, slice(None))]
+        # only syntactic sugar differences (__getitem__ gets *exact* same key)
+        # agg_arr[lg1, slice(None)]
+        # agg_arr[lg1, :]
 
 
 def test_filter_on_group_agg(array):
@@ -2543,8 +2547,8 @@ def test_filter_on_group_agg(array):
     # using a named LGroup
     g1 = b[b_group1] >> 'g1'
     agg_arr = array.sum(a, c).sum(b=(g1, b_group2, b_group3, all_b))
-    with must_warn(FutureWarning, msg=group_as_aggregated_label_msg(g1)):
-        assert agg_arr.filter(b=g1).shape == (6,)
+    with must_raise(ValueError, msg=_group_as_aggregated_label_msg(g1)):
+        agg_arr.filter(b=g1)
 
     # Note that agg_arr.filter(b=(g1,)) does NOT work. It might be a
     # little confusing for users, because agg_arr[(g1,)] works but it is
@@ -2567,8 +2571,9 @@ def test_filter_on_group_agg(array):
     # assert bya.filter(a=':17').shape == (12, 2, 6)
 
     bya = array.sum(a=(a0to5_named, 5, a6to13, a14plus))
-    with must_warn(FutureWarning, msg=group_as_aggregated_label_msg(a0to5_named)):
-        assert bya.filter(a=a0to5_named).shape == (12, 2, 6)
+    msg = _group_as_aggregated_label_msg(a0to5_named)
+    with must_raise(ValueError, msg=msg):
+        bya.filter(a=a0to5_named)
 
 
 def test_sum_several_lg_groups(array):
@@ -2583,12 +2588,11 @@ def test_sum_several_lg_groups(array):
 
     # the result is indexable
     # 1.a) by LGroup
-    msg1 = group_as_aggregated_label_msg(lg1)
-    with must_warn(FutureWarning, msg=msg1):
-        assert agg_arr.filter(b=lg1).shape == (19, 2, 6)
-    msg2 = group_as_aggregated_label_msg(lg2)
-    with must_warn(FutureWarning, msg=(msg1, msg2), check_file=False):
-        assert agg_arr.filter(b=(lg1, lg2)).shape == (19, 2, 2, 6)
+    msg = _group_as_aggregated_label_msg(lg1)
+    with must_raise(ValueError, msg=msg):
+        agg_arr.filter(b=lg1)
+    with must_raise(ValueError, msg=msg):
+        agg_arr.filter(b=(lg1, lg2))
 
     # 1.b) by string (name of groups)
     assert agg_arr.filter(b='lg1').shape == (19, 2, 6)