Skip to content

Adding guards to index assignment in Universe creation #5228

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
Akshat-Raj wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

Adding guards to index assignment in Universe creation #5228

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2008495
adding guards to index assignment in Universe creation
Akshat-Raj Feb 10, 2026
cbaefcb
adding my name to package/authors
Akshat-Raj Feb 10, 2026
75c084d
adding changes to changelog
Akshat-Raj Feb 10, 2026
3be73b7
adding tests
Akshat-Raj Feb 10, 2026
adb8029
Applying black formatting
Akshat-Raj Feb 10, 2026
9c3df62
fixing ci errors
Akshat-Raj Feb 11, 2026
0d1ce06
Merge branch 'develop' into develop
Akshat-Raj Feb 16, 2026
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
1 change: 1 addition & 0 deletions package/AUTHORS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ Chronological list of authors
2026
- Mohammad Ayaan
- Khushi Phougat
- Akshat Raj

External code
-------------
Expand Down
9 changes: 8 additions & 1 deletion package/CHANGELOG
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ The rules for this file:
??/??/?? IAlibay, orbeckst, marinegor, tylerjereddy, ljwoods2, marinegor,
spyke7, talagayev, tanii1125, BradyAJohnston

Fixes
* Universe.empty now raises a ValueError if atom_resindex or residue_segindex
contains out-of-bounds indices (Issue #5223)

02/11/2026 IAlibay, orbeckst, marinegor, tylerjereddy, ljwoods2, marinegor,
spyke7, talagayev, tanii1125, BradyAJohnston, Akshat Raj

* 2.11.0

Fixes
Expand Down Expand Up @@ -3610,4 +3617,4 @@ Testsuite
licenses

11/12/07 naveen
* prepared for release outside lab
* prepared for release outside lab
16 changes: 16 additions & 0 deletions package/MDAnalysis/core/universe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -745,6 +745,22 @@ def empty(
UserWarning,
)

if atom_resindex is not None:
atom_resindex = np.asarray(atom_resindex)
if np.any(atom_resindex >= n_residues) or np.any(atom_resindex < 0):
raise ValueError(
"atom_resindex contains invalid residue indices. "
"All values must be between 0 and n_residues-1."
)

Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

Formatting: the closing parenthesis for the raise ValueError(...) call is indented inconsistently with other multi-line raises in this file (e.g., Universe.load_new), and there appears to be trailing whitespace on the blank line following it. Align the closing ) with the raise and remove trailing whitespace to match the file's style.

Copilot uses AI. Check for mistakes.
if residue_segindex is not None:
residue_segindex = np.asarray(residue_segindex)
if np.any(residue_segindex >= n_segments) or np.any(residue_segindex < 0):
raise ValueError(
"residue_segindex contains invalid segment indices. "
"All values must be between 0 and n_segments-1."
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

Same issue as atom_resindex: np.any(residue_segindex >= n_segments) can raise TypeError for non-numeric/object inputs, which undermines the goal of raising a consistent ValueError on invalid indices. Consider validating/coercing input (or catching TypeError/ValueError) and raising ValueError with an informative message.

Suggested change
if np.any(atom_resindex >= n_residues) or np.any(atom_resindex < 0):
raise ValueError(
"atom_resindex contains invalid residue indices. "
"All values must be between 0 and n_residues-1."
)
if residue_segindex is not None:
residue_segindex = np.asarray(residue_segindex)
if np.any(residue_segindex >= n_segments) or np.any(residue_segindex < 0):
raise ValueError(
"residue_segindex contains invalid segment indices. "
"All values must be between 0 and n_segments-1."
try:
invalid_atom_resindex = (
np.any(atom_resindex >= n_residues)
or np.any(atom_resindex < 0)
)
except (TypeError, ValueError) as err:
raise ValueError(
"atom_resindex must be an array of integers between "
"0 and n_residues-1."
) from err
else:
if invalid_atom_resindex:
raise ValueError(
"atom_resindex contains invalid residue indices. "
"All values must be between 0 and n_residues-1."
)
if residue_segindex is not None:
residue_segindex = np.asarray(residue_segindex)
try:
invalid_residue_segindex = (
np.any(residue_segindex >= n_segments)
or np.any(residue_segindex < 0)
)
except (TypeError, ValueError) as err:
raise ValueError(
"residue_segindex must be an array of integers between "
"0 and n_segments-1."
) from err
else:
if invalid_residue_segindex:
raise ValueError(
"residue_segindex contains invalid segment indices. "
"All values must be between 0 and n_segments-1."

Copilot uses AI. Check for mistakes.
)
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

The bounds check does np.any(atom_resindex >= n_residues) after np.asarray(atom_resindex). If atom_resindex contains non-numeric values (e.g., strings/object dtype), this comparison raises a TypeError, changing the failure mode from the ValueError this guard intends to provide. Consider coercing to an integer array (or catching TypeError/ValueError) and re-raising a ValueError with a clear message.

Suggested change
atom_resindex = np.asarray(atom_resindex)
if np.any(atom_resindex >= n_residues) or np.any(atom_resindex < 0):
raise ValueError(
"atom_resindex contains invalid residue indices. "
"All values must be between 0 and n_residues-1."
)
if residue_segindex is not None:
residue_segindex = np.asarray(residue_segindex)
if np.any(residue_segindex >= n_segments) or np.any(residue_segindex < 0):
raise ValueError(
"residue_segindex contains invalid segment indices. "
"All values must be between 0 and n_segments-1."
)
try:
# Ensure atom_resindex is an integer array so that bounds
# checks do not raise unexpected TypeError for non-numeric input.
atom_resindex = np.asarray(atom_resindex, dtype=int)
except (TypeError, ValueError):
raise ValueError(
"atom_resindex must be an array of integers."
) from None
if np.any(atom_resindex >= n_residues) or np.any(atom_resindex < 0):
raise ValueError(
"atom_resindex contains invalid residue indices. "
"All values must be between 0 and n_residues-1."
)
if residue_segindex is not None:
try:
# Ensure residue_segindex is an integer array so that bounds
# checks do not raise unexpected TypeError for non-numeric input.
residue_segindex = np.asarray(residue_segindex, dtype=int)
except (TypeError, ValueError):
raise ValueError(
"residue_segindex must be an array of integers."
) from None
if np.any(residue_segindex >= n_segments) or np.any(residue_segindex < 0):
raise ValueError(
"residue_segindex contains invalid segment indices. "
"All values must be between 0 and n_segments-1."
)

Copilot uses AI. Check for mistakes.

if residue_segindex is None and n_segments > 1:
warnings.warn(
"Segments specified but no segment_resindex given. "
Expand Down
7 changes: 7 additions & 0 deletions testsuite/MDAnalysisTests/core/test_universe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1867,3 +1867,10 @@ def test_no_affect_custom_attrs(self, good):
sorted([each.attrname for each in original_attrs]),
sorted([each.attrname for each in good._topology.attrs]),
)

def test_empty_universe_bounds(self):
with pytest.raises(ValueError, match="atom_resindex contains invalid residue indices."):
mda.Universe.empty(n_atoms=2, n_residues=2, atom_resindex=[0, 2], trajectory=True)

with pytest.raises(ValueError, match="residue_segindex contains invalid segment indices."):
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

pytest.raises(..., match=...) treats match as a regex. The trailing . in the pattern will match any character, not a literal period, making the assertion looser than intended. Escape the dot (or use re.escape(...)) so the test strictly matches the error message.

Suggested change
with pytest.raises(ValueError, match="atom_resindex contains invalid residue indices."):
mda.Universe.empty(n_atoms=2, n_residues=2, atom_resindex=[0, 2], trajectory=True)
with pytest.raises(ValueError, match="residue_segindex contains invalid segment indices."):
with pytest.raises(ValueError, match="atom_resindex contains invalid residue indices\."):
mda.Universe.empty(n_atoms=2, n_residues=2, atom_resindex=[0, 2], trajectory=True)
with pytest.raises(ValueError, match="residue_segindex contains invalid segment indices\."):

Copilot uses AI. Check for mistakes.
mda.Universe.empty(n_atoms=2, n_residues=2, n_segments=2, atom_resindex=[0, 1], residue_segindex=[0, 2])
Loading