Skip to content
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.

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

Fixed high dimensional GroupBase indexing. #4656

Merged
merged 3 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
2 changes: 1 addition & 1 deletion package/AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ Chronological list of authors
- Kurt McKee
- Fabian Zills
- Laksh Krishna Sharma

- Matthew Davies


External code
Expand Down
1 change: 1 addition & 0 deletions package/CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ The rules for this file:
* 2.8.0

Fixes
* Sanitise and catch higher dimensional indexing in GroupBase (Issue #4647)
* Do not raise an Error reading H5MD files with datasets like
`observables/<particle>/<property>` (part of Issue #4598, PR #4615)
* Fix failure in double-serialization of TextIOPicklable file reader.
Expand Down
10 changes: 10 additions & 0 deletions package/MDAnalysis/core/groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,16 @@ def __getitem__(self, item):
# hack to make lists into numpy arrays
# important for boolean slicing
item = np.array(item)

# correctly flatten oddly shaped, but 1d arrays
# raises issues if flattening already suitable arrays
if isinstance(item, np.ndarray) and item.ndim > 1: # ignore slices
item = np.squeeze(item)
# disallow high dimensional indexing.
# this doesnt stop the underlying issue
if item.ndim > 1:
raise IndexError('Group index must be 1d')

# We specify _derived_class instead of self.__class__ to allow
# subclasses, such as UpdatingAtomGroup, to control the class
# resulting from slicing.
Expand Down
19 changes: 19 additions & 0 deletions testsuite/MDAnalysisTests/core/test_atomgroup.py
Original file line number Diff line number Diff line change
Expand Up @@ -1592,6 +1592,25 @@ def test_index_advancedslice(self, universe):
"an AtomGroup")
assert_equal(ag[1], ag[-1], "advanced slicing does not preserve order")

@pytest.mark.parametrize('shape', [(-1, 1), (1, -1)])
def test_1d_index_invariance(self, universe, shape):
u = universe
ag1 = u.atoms[np.arange(5)]
ag2 = u.atoms[np.arange(5).reshape(shape)]

# without correct sanitising, n_atoms is 1 when reshaped 'incorrectly'
assert ag2.n_atoms == 5, ("indexing with a 1d array was not "
"invarient of row vs column order")
assert_equal(ag1, ag2), ("indexing with a 1d array was not "
"invarient of row vs column order")

def test_2d_indexing_caught(self, universe):
u = universe
index_2d = [[1, 2, 3],
[4, 5, 6]]
with pytest.raises(IndexError):
u.atoms[index_2d]

@pytest.mark.parametrize('sel', (np.array([True, False, True]),
[True, False, True]))
def test_boolean_indexing_2(self, universe, sel):
Expand Down
Loading