Skip to content

Fill test coverage gaps and fix polyhedra_from_atom_indices bug#16

Open
bjmorgan wants to merge 3 commits intomainfrom
fill-test-gaps
Open

Fill test coverage gaps and fix polyhedra_from_atom_indices bug#16
bjmorgan wants to merge 3 commits intomainfrom
fill-test-gaps

Conversation

@bjmorgan
Copy link
Owner

@bjmorgan bjmorgan commented Mar 3, 2026

Summary

  • Add 42 behavioural tests covering previously untested methods across all major modules (utils, orientation_parameters, CoordinationPolyhedron, PolyhedraRecipe, Configuration, Trajectory)
  • Fix variable shadowing bug in polyhedra_from_atom_indices where the loop variable overwrote the vertex_atoms parameter, causing only the first polyhedron to receive correct vertices (fixes Bug: polyhedra_from_atom_indices shadows vertex_atoms parameter in loop #15)
  • New tests prefer real pymatgen objects over mocks where practical

bjmorgan added 3 commits March 3, 2026 22:02
Add 42 behavioural tests across all major modules, covering previously
untested methods and edge cases:

- utils: prune_neighbour_list, lattice_mc_string
- orientation_parameters: projection_xyz, oct_rotational_order_parameter
- CoordinationPolyhedron: label/set_label, equal_vertices, __eq__,
  best_fit_geometry, volume, edge_graph, faces,
  minimum_image_vertex_coordinates, intersection, neighbours
- PolyhedraRecipe: polyhedra_from_nearest_neighbours,
  polyhedra_from_closest_centre, polyhedra_from_atom_indices,
  end-to-end find_polyhedra for nearest neighbours and closest centre
- Configuration: polyhedra_by_label with list, face_sharing_neighbour_list,
  to_lattice_mc
- Trajectory: from_structures, __add__

New tests prefer real pymatgen objects over mocks where practical.

Fix variable shadowing bug in polyhedra_from_atom_indices where the loop
variable overwrote the vertex_atoms parameter, causing only the first
polyhedron to receive correct vertices (see #15).
…e_cutoff

Address code review feedback on PR #16:

- Test callable, list[int], and invalid-type branches of
  generator_from_atom_argument
- Add behavioural test for polyhedra_from_distance_cutoff with real
  atoms verifying cutoff filtering
- Add regression docstring on test_correct_assignment_from_explicit_indices
  linking to issue #15
Patch release for the polyhedra_from_atom_indices bugfix (#15).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: polyhedra_from_atom_indices shadows vertex_atoms parameter in loop

1 participant