Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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: 2 additions & 0 deletions corelib/src/libs/SireMol/molid.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,8 @@ QList<MolNum> MolIdx::map(const Molecules &molecules) const
molnums.append(it.key());
break;
}

--i;
}

BOOST_ASSERT(not molnums.isEmpty());
Expand Down
58 changes: 56 additions & 2 deletions corelib/src/libs/SireSearch/idengine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@
#include "SireBase/booleanproperty.h"
#include "SireBase/parallel.h"

#include "SireMol/atomelements.h"
#include "SireMol/atomcoords.h"
#include "SireMol/atomelements.h"
#include "SireMol/core.h"
#include "SireMol/iswater.h"

Expand Down Expand Up @@ -1155,9 +1155,63 @@ SelectResult IDIndexEngine::searchMolIdx(const SelectResult &mols, const SelectR
{
QList<Molecule> matches;

int idx = 0;
int count = context.listCount();

if (_is_single_value(this->vals))
{
// For a single index value, apply Python-style negative-index mapping
// and do a strict bounds check. Out-of-bounds returns no match,
// consistent with residx/atomidx behaviour.
// We read the raw start value from RangeValue directly, bypassing the
// _to_single_value helper which maps against INT_MAX and corrupts
// negative indices.
auto rv = boost::get<RangeValue>(this->vals[0]);

if (not rv.start)
return SelectResult(matches);

int v = *rv.start;

if (v < 0)
v = count + v;

if (v < 0 or v >= count)
return SelectResult(matches);

int idx = 0;

for (const auto &mol : context)
{
if (idx == v)
{
if (&mols == &context)
{
matches.append(mol->molecule());
}
else
{
const auto molnum = mol->data().number();

for (const auto &m : mols)
{
if (m->data().number() == molnum)
{
matches.append(m->molecule());
break;
}
}
}
break;
}

idx += 1;
}

return SelectResult(matches);
}

int idx = 0;

for (const auto &mol : context)
{
if (this->match(idx, count))
Expand Down
4 changes: 4 additions & 0 deletions doc/source/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ organisation on `GitHub <https://github.com/openbiosim/sire>`__.

* Map the end-state ``element`` property when performing hydrogen mass repartitioning on perturbable molecules.

* Fixed out-of-bounds ``molidx`` searches silently returning the last molecule instead of
raising a ``KeyError``. Out-of-bounds positive and negative single-index values now
behave consistently with ``residx`` and ``atomidx``.

`2025.4.0 <https://github.com/openbiosim/sire/compare/2025.3.0...2025.4.0>`__ - February 2026
---------------------------------------------------------------------------------------------

Expand Down
5 changes: 5 additions & 0 deletions ruff.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[lint]
ignore = ["E402"]

[lint.per-file-ignores]
"tests/**" = ["F841"]
32 changes: 28 additions & 4 deletions tests/mol/test_complex_indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ def test_search_terms(ala_mols):
check_mass = mols[0][0].mass().value()

atoms = mols[0][
f"atom mass >= {check_mass-0.001} and atom mass <= {check_mass+0.001}"
f"atom mass >= {check_mass - 0.001} and atom mass <= {check_mass + 0.001}"
]

assert len(atoms) > 0
Expand All @@ -460,7 +460,7 @@ def test_search_terms(ala_mols):
check_charge = mols[0][1].charge().value()

atoms = mols[0][
f"atom charge >= {check_charge-0.001} and atom charge <= {check_charge+0.001}"
f"atom charge >= {check_charge - 0.001} and atom charge <= {check_charge + 0.001}"
]

assert len(atoms) > 0
Expand Down Expand Up @@ -527,8 +527,6 @@ def test_in_searches(ala_mols):
def test_with_searches(ala_mols):
mols = ala_mols

import sire as sr

for mol in mols["molecules with count(atoms) >= 3"]:
assert mol.num_atoms() >= 3

Expand Down Expand Up @@ -593,3 +591,29 @@ def test_count_searches(ala_mols):

mol = mols["molecules with count(residues) == 3"]
assert mol.num_residues() == 3


def test_oob_molidx(ala_mols):
"""
Regression test for issue #286: out-of-bounds molidx should raise KeyError,
not silently return the last molecule.
"""
mols = ala_mols

n = mols.num_molecules()

# Valid boundary indices should work
assert mols["molidx 0"] == mols[0]
assert mols[f"molidx {n - 1}"] == mols[-1]
assert mols["molidx -1"] == mols[-1]

# Out-of-bounds positive index must raise KeyError
with pytest.raises(KeyError):
mols[f"molidx {n}"]

with pytest.raises(KeyError):
mols["molidx 10000"]

# Out-of-bounds negative index must raise KeyError
with pytest.raises(KeyError):
mols[f"molidx -{n + 1}"]