Skip to content

allow ; in get/setitem #983

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

allow ; in get/setitem #983

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
11 changes: 11 additions & 0 deletions doc/source/changes/version_0_34.rst.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,17 @@ Backward incompatible changes
New features
^^^^^^^^^^^^

* allow ";" in __getitem__ and __setitem__ strings, making it possible to specify keys for several axes in a single
string (closes :issue:`904`). For example:

>>> arr = ndtest((2, 3))
>>> arr
a\b b0 b1 b2
a0 0 1 2
a1 3 4 5
>>> arr['a1;b2']
5

* added a feature (see the :ref:`miscellaneous section <misc>` for details). It works on :ref:`api-axis` and
:ref:`api-group` objects.

Expand Down
6 changes: 6 additions & 0 deletions larray/core/axis.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2811,6 +2811,12 @@ def _key_to_axis_indices_dict(self, key):
"""
from .array import Array

if isinstance(key, str) and ';' in key:
# FIXME: it would be more logical to use _to_keys(key) instead but this breaks
# the "target an aggregate with its key" feature (test_getitem_on_group_agg).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry but I don't understand what you mean by "target an aggregate with its key" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that the two last lines both work:

>>> arr = ndtest(4)
>>> arr
a  a0  a1  a2  a3
    0   1   2   3
>>> a = arr.a
>>> g1 = a['a1,a3'] >> 'g1'
>>> g2 = a['a0,a2']
>>> agg = arr.sum((g1, g2))
>>> agg
a  g1  a0,a2
    4      2
>>> agg[g1]
4
>>> agg[g2]
2

This is an obscure feature of LArray which I thought users would want to use but was a pain to implement and "keep working over the years". It makes the code ugly in a few places and costs a bit of performance too... and AFAIK nobody uses it so we should scrap it I think.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. In that case, add an issue for that (or we will rapidly completely forget about it). Thx.

# It might be a good thing though as it costs so much for so little use.
# key = _to_keys(key)
key = tuple(key.split(';'))
if isinstance(key, dict):
# key axes could be strings or axis references and we want real axes
key = tuple(self[axis][axis_key] for axis, axis_key in key.items())
Expand Down
8 changes: 8 additions & 0 deletions larray/tests/test_array.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,14 @@ def test_getitem_guess_axis(array):
res = arr['b[l1]']
assert_array_equal(res, arr.data[:, 0])

# several axes in same string (guess axis)
res = arr['l0;l2']
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update the tutorial with a similar example?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

assert res == 1

# several axes in same string (with axis information)
res = arr['a[l1];b[l2]']
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

assert res == 3


def test_getitem_positional_group(array):
raw = array.data
Expand Down