Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gdementen merged 1 commit into from
Dec 21, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions doc/source/changes/version_0_35.rst.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
42 changes: 24 additions & 18 deletions larray/core/axis.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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__
Expand Down Expand Up @@ -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
Expand Down
106 changes: 55 additions & 51 deletions larray/tests/test_array.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 #
# ================== #
Expand Down Expand Up @@ -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):
Expand All @@ -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):
Expand All @@ -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
Expand All @@ -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):
Expand All @@ -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)
Expand Down