test_get_equivalent_points_functional tests wrong property

In test_analyse_symmetry.py, test_get_equivalent_points_functional
purports to do the same test as
test_get_equivalent_points
except via the functional entry point rather than the Symmetry class. However the function calls get_arg_equivalent_sites internally and so returns indices rather than positions, so that's a category mistake.

Not sure whether the function or the test is wrong, though, so just flagging it here.

@samwaseda what do you think?

Actionable comments posted: 1

[!CAUTION]
Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/test_analyse_symmetry.py (1)

332-339: ⚠️ Potential issue | 🟠 Major

Fix the functional get_equivalent_points test to assert equivalence-class IDs, not coordinates.

test_get_equivalent_points() calls .generate_equivalent_points() and correctly asserts the minimum distance is 0 (the point lands on [0.51, 0.5, 0]). However, test_get_equivalent_points_functional() calls the functional get_equivalent_points(), which delegates to .get_arg_equivalent_sites() and returns equivalence-class IDs, not coordinates. The test incorrectly treats the IDs as coordinates, computing norm(arr - [0.51, 0.5, 0], axis=-1). When arr = [0] (the equivalence class ID), this norm equals 0.7142128534267638—exactly the norm of the coordinate vector itself—making the test pass despite checking the wrong output type.

Either use .generate_equivalent_points() to match the instance-method test, or assert the correct equivalence-class IDs.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_analyse_symmetry.py` around lines 332 - 339, The functional test
calls get_equivalent_points (which delegates to get_arg_equivalent_sites) and
thus returns equivalence-class IDs, not coordinates; update
test_get_equivalent_points_functional to assert the returned IDs (e.g., that arr
equals the expected class id like [0] or contains 0) instead of computing
distances to coordinate [0.51, 0.5, 0], or alternatively call
generate_equivalent_points (matching the instance-method test) if you intended
to compare coordinates; adjust the assertion to use get_equivalent_points (IDs)
or generate_equivalent_points (coordinates) accordingly.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/test_analyse_symmetry.py`:
- Around line 451-457: The test test_error currently allows either
stk.common.SymmetryError or SpglibError which masks regressions in exception
wrapping; update the assertion in tests/test_analyse_symmetry.py so the with
self.assertRaises only expects stk.common.SymmetryError (remove SpglibError),
ensuring stk.analyse.get_symmetry raises the wrapped SymmetryError as documented
and the docstring remains accurate.

---

Outside diff comments:
In `@tests/test_analyse_symmetry.py`:
- Around line 332-339: The functional test calls get_equivalent_points (which
delegates to get_arg_equivalent_sites) and thus returns equivalence-class IDs,
not coordinates; update test_get_equivalent_points_functional to assert the
returned IDs (e.g., that arr equals the expected class id like [0] or contains
0) instead of computing distances to coordinate [0.51, 0.5, 0], or alternatively
call generate_equivalent_points (matching the instance-method test) if you
intended to compare coordinates; adjust the assertion to use
get_equivalent_points (IDs) or generate_equivalent_points (coordinates)
accordingly.

Originally posted by @coderabbitai[bot] in https://github.com/pyiron/structuretoolkit/pull/144#pullrequestreview-4092362317

Start with tests/test_analyse_symmetry.py around test_get_equivalent_points_functional and inspect the functional entry point in src/structuretoolkit/analyse/init.py, especially its call to get_arg_equivalent_sites. Run the focused symmetry tests to confirm whether the function returns equivalence-class IDs or coordinates. Done means the functional test asserts the value returned by that entry point rather than passing with an incorrect distance calculation.