diff --git a/doc/source/changes/version_0_35.rst.inc b/doc/source/changes/version_0_35.rst.inc index 8b5317684..ce89521b2 100644 --- a/doc/source/changes/version_0_35.rst.inc +++ b/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 ^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/larray/core/axis.py b/larray/core/axis.py index ef4b0109b..ed1c52b8f 100644 --- a/larray/core/axis.py +++ b/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 diff --git a/larray/tests/test_array.py b/larray/tests/test_array.py index 288498ca1..be2118978 100644 --- a/larray/tests/test_array.py +++ b/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)