Throw python exceptions by LecrisUT · Pull Request #579 · spglib/spglib

LecrisUT · 2025-05-10T19:09:04Z

TODO:

Make the library throw exceptions
Add a compatibility for the old error handling
Add more checks in the C++ code

Depends-on: #578
Closes #582

codecov · 2025-05-10T19:12:01Z

Codecov Report

❌ Patch coverage is 56.25000% with 84 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.05%. Comparing base (cb11bf2) to head (4bd37ad).
⚠️ Report is 52 commits behind head on develop.

Files with missing lines	Patch %	Lines
python/py_bindings.cpp	46.66%	74 Missing and 6 partials ⚠️
python/py_bindings.h	25.00%	3 Missing ⚠️
python/_spglib.cpp	97.29%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #579      +/-   ##
===========================================
- Coverage    74.51%   74.05%   -0.46%     
===========================================
  Files           26       27       +1     
  Lines         7899     8018     +119     
  Branches      1635     1646      +11     
===========================================
+ Hits          5886     5938      +52     
- Misses        1534     1603      +69     
+ Partials       479      477       -2

Flag	Coverage Δ
c_api	`69.45% <100.00%> (ø)`
fortran_api	`52.96% <0.00%> (ø)`
python_api	`67.18% <55.72%> (-0.54%)`	⬇️
unit_tests	`11.68% <0.00%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

LecrisUT · 2025-11-09T21:49:21Z

There were still some areas that I didn't manage to add proper checks on the cpp side, but it should be a good enough approach for now.

python/py_bindings.h

ChangeLog.md

lan496 · 2025-11-19T00:44:26Z

Some existing tests start to raise SpglibError and result in CI failure

lan496 · 2025-11-19T00:46:26Z

@atztogo, can you also take a look at this PR?

LecrisUT · 2025-11-19T04:46:22Z

Some existing tests start to raise SpglibError and result in CI failure

Ah, it's trivial. We have changed what the exception was being raised in that test and I forgot to adapt it

Signed-off-by: Cristian Le <[email protected]>

Encountered implicit conversion issues. This can probably be relaxed when all arguments are typed accordingly Signed-off-by: Cristian Le <[email protected]>

- Simplifies the conversions - Embeds the array size checks Signed-off-by: Cristian Le <[email protected]>

For some reason it doesn't pick it up when trying to use it

Signed-off-by: Cristian Le <[email protected]>

atztogo

LGTM. Thanks @LecrisUT!

lan496

LGTM

siteshwar · 2025-11-20T08:47:33Z

There were some deprecation warnings reported by OpenScanHub that might be good to fix:

Error: COMPILER_WARNING ([CWE-477](https://cwe.mitre.org/data/definitions/477.html)): [[#def1]](https://openscanhub.fedoraproject.org/task/82755/log/added.html#def1)
spglib-2.6.0/python/py_bindings.cpp: scope_hint: In function ‘pybind11::dict spglib::layer_dataset(const Lattice&, const Positions&, const AtomTypes&, pybind11::int_, pybind11::float_)’
spglib-2.6.0/python/py_bindings.cpp:366:41: warning[-Wdeprecated-declarations]: ‘SpglibDataset* spg_get_layer_dataset(const double (*)[3], const double (*)[3], const int*, int, int, double)’ is deprecated: Experimental interface. May be removed in next major release.
#  366 |     auto dataset = spg_get_layer_dataset(lattice.data(), positions.data(),
#      |                    ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#  367 |                                          atom_types.data(), atom_types.n_atoms,
#      |                                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#  368 |                                          aperiodic_dir, symprec);
#      |                                          ~~~~~~~~~~~~~~~~~~~~~~~
spglib-2.6.0/python/py_bindings.cpp:4: included_from: Included from here.
spglib-2.6.0/include/spglib.h:212:24: note: declared here
#  212 | SPG_API SpglibDataset *spg_get_layer_dataset(
#      |                        ^~~~~~~~~~~~~~~~~~~~~
#  364|                                  AtomTypes const &atom_types,
#  365|                                  py::int_ aperiodic_dir, py::float_ symprec) {
#  366|->     auto dataset = spg_get_layer_dataset(lattice.data(), positions.data(),
#  367|                                            atom_types.data(), atom_types.n_atoms,
#  368|                                            aperiodic_dir, symprec);

Error: COMPILER_WARNING ([CWE-477](https://cwe.mitre.org/data/definitions/477.html)): [[#def2]](https://openscanhub.fedoraproject.org/task/82755/log/added.html#def2)
spglib-2.6.0/python/py_bindings.cpp: scope_hint: In function ‘pybind11::int_ spglib::symmetry(Rotations&, Translations&, const Lattice&, const Positions&, const AtomTypes&, pybind11::float_, pybind11::float_)’
spglib-2.6.0/python/py_bindings.cpp:492:34: warning[-Wdeprecated-declarations]: ‘int spgat_get_symmetry(int (*)[3][3], double (*)[3], int, const double (*)[3], const double (*)[3], const int*, int, double, double)’ is deprecated: Use the variables from SpglibDataset (rotations, translations)
#  492 |     auto val = spgat_get_symmetry(rotations.data(), translations.data(),
#      |                ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#  493 |                                   rotations.n_operations, lattice.data(),
#      |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#  494 |                                   positions.data(), atom_types.data(),
#      |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#  495 |                                   atom_types.n_atoms, symprec, angle_tolerance);
#      |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
spglib-2.6.0/include/spglib.h:254:13: note: declared here
#  254 | SPG_API int spgat_get_symmetry(int rotation[][3][3], double translation[][3],
#      |             ^~~~~~~~~~~~~~~~~~
#  490|                             AtomTypes const &atom_types, py::float_ symprec,
#  491|                             py::float_ angle_tolerance) {
#  492|->     auto val = spgat_get_symmetry(rotations.data(), translations.data(),
#  493|                                     rotations.n_operations, lattice.data(),
#  494|                                     positions.data(), atom_types.data(),

Error: COMPILER_WARNING ([CWE-477](https://cwe.mitre.org/data/definitions/477.html)): [[#def3]](https://openscanhub.fedoraproject.org/task/82755/log/added.html#def3)
spglib-2.6.0/python/py_bindings.cpp: scope_hint: In function ‘pybind11::int_ spglib::hall_number_from_symmetry(const Rotations&, const Translations&, pybind11::float_)’
spglib-2.6.0/python/py_bindings.cpp:658:49: warning[-Wdeprecated-declarations]: ‘int spg_get_hall_number_from_symmetry(const int (*)[3][3], const double (*)[3], int, double)’ is deprecated: Use the variable from SpglibSpacegroupType instead (hall_number)
#  658 |     auto val = spg_get_hall_number_from_symmetry(
#      |                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
#  659 |         rotations.data(), translations.data(), rotations.n_operations, symprec);
#      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
spglib-2.6.0/include/spglib.h:327:13: note: declared here
#  327 | SPG_API int spg_get_hall_number_from_symmetry(int const rotation[][3][3],
#      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#  656|                                              Translations const &translations,
#  657|                                              py::float_ symprec) {
#  658|->     auto val = spg_get_hall_number_from_symmetry(
#  659|           rotations.data(), translations.data(), rotations.n_operations, symprec);
#  660|       if (val == 0) throw Spglib_classic_exception();

LecrisUT · 2025-11-20T20:34:15Z

Those are first-party deprecations (stuff we deprecated but we still offer the python interface).

LecrisUT force-pushed the feat/python-checks branch 3 times, most recently from 9a8373e to df4be1d Compare May 12, 2025 11:50

This was referenced May 29, 2025

Incompatible function arguments in develop version #582

Closed

Release v2.7 #583

Closed

LecrisUT added this to the 2.7 milestone Jun 2, 2025

LecrisUT mentioned this pull request Jun 20, 2025

Niggli reduction before finding hall number from symmetry operations #589

Merged

LecrisUT mentioned this pull request Jul 22, 2025

spgat_standardize_cell sometimes does not extend types #599

Open

LecrisUT force-pushed the feat/python-checks branch from df4be1d to 41c6c8a Compare July 22, 2025 12:37

atztogo mentioned this pull request Oct 30, 2025

Release 2.6.1 #611

Closed

LecrisUT force-pushed the feat/python-checks branch 2 times, most recently from 8c22839 to 80aa052 Compare November 9, 2025 21:24

LecrisUT marked this pull request as ready for review November 9, 2025 21:45

LecrisUT changed the title ~~WIP: Throw python exceptions~~ Throw python exceptions Nov 9, 2025

LecrisUT force-pushed the feat/python-checks branch from 8125d0c to 39437e1 Compare November 9, 2025 21:47

LecrisUT requested a review from lan496 November 9, 2025 21:49

LecrisUT assigned lan496 Nov 9, 2025

lan496 force-pushed the feat/python-checks branch from 39437e1 to 283348e Compare November 11, 2025 05:56

lan496 reviewed Nov 16, 2025

View reviewed changes

python/py_bindings.h Show resolved Hide resolved

lan496 added the breaking-changes Changes that are incompatible with previous release label Nov 16, 2025

lan496 reviewed Nov 16, 2025

View reviewed changes

ChangeLog.md Outdated Show resolved Hide resolved

LecrisUT requested a review from lan496 November 17, 2025 17:22

LecrisUT force-pushed the feat/python-checks branch from 283348e to 3d8691f Compare November 17, 2025 17:22

lan496 requested a review from atztogo November 19, 2025 00:46

Use f-strings

d778107

Signed-off-by: Cristian Le <[email protected]>

LecrisUT mentioned this pull request Nov 19, 2025

Split the spglib.spglib monolith into individual submodules #620

Merged

LecrisUT added 11 commits November 19, 2025 13:32

Prepare to use proper exceptions

59d1c72

Signed-off-by: Cristian Le <[email protected]>

Throw the spglib errors

77f5f5d

Signed-off-by: Cristian Le <[email protected]>

Temporarily set the py::arg for the overloaded functions

af4b641

Encountered implicit conversion issues. This can probably be relaxed when all arguments are typed accordingly Signed-off-by: Cristian Le <[email protected]>

[WIP] Introduce typed array

544389d

- Simplifies the conversions - Embeds the array size checks Signed-off-by: Cristian Le <[email protected]>

Use the typed arrays

f3f09fa

Fixup the error setting

4e39d01

Fixup AtomTypes

fb4c782

More explicit classes

54cc3e4

Revert using Magmoms

259563b

For some reason it doesn't pick it up when trying to use it

Fixup pre-commit

d1f9076

Enforce some type normalizations

30d47dd

LecrisUT force-pushed the feat/python-checks branch from 3d8691f to f080815 Compare November 19, 2025 13:25

LecrisUT added 8 commits November 19, 2025 14:36

Add a SpglibError base exception

1b763d5

Signed-off-by: Cristian Le <[email protected]>

Free the SpglibError usage

18c7c0f

Recast the error types so far to SpglibError

8f8cfc6

Define a cpp exception type

f0a78e6

Throw cpp errors as SpglibError

4fc2395

Add more documentation about exception feature

db1b61d

Review missing exceptions in py_bindings.cpp

0b103a3

Signed-off-by: Cristian Le <[email protected]>

fixup! Recast the error types so far to SpglibError

4bd37ad

Signed-off-by: Cristian Le <[email protected]>

LecrisUT force-pushed the feat/python-checks branch from f080815 to 4bd37ad Compare November 19, 2025 13:38

atztogo approved these changes Nov 20, 2025

View reviewed changes

lan496 approved these changes Nov 20, 2025

View reviewed changes

lan496 merged commit 170ff8c into spglib:develop Nov 20, 2025
47 of 50 checks passed

LecrisUT deleted the feat/python-checks branch November 20, 2025 13:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Throw python exceptions#579

Throw python exceptions#579
lan496 merged 20 commits intospglib:developfrom
LecrisUT:feat/python-checks

LecrisUT commented May 10, 2025 •

edited

Loading

Uh oh!

codecov bot commented May 10, 2025 •

edited

Loading

Uh oh!

LecrisUT commented Nov 9, 2025

Uh oh!

Uh oh!

Uh oh!

lan496 commented Nov 19, 2025

Uh oh!

lan496 commented Nov 19, 2025

Uh oh!

LecrisUT commented Nov 19, 2025

Uh oh!

atztogo left a comment

Uh oh!

lan496 left a comment

Uh oh!

Uh oh!

siteshwar commented Nov 20, 2025

Uh oh!

LecrisUT commented Nov 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

LecrisUT commented May 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented May 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

LecrisUT commented Nov 9, 2025

Uh oh!

Uh oh!

Uh oh!

lan496 commented Nov 19, 2025

Uh oh!

lan496 commented Nov 19, 2025

Uh oh!

LecrisUT commented Nov 19, 2025

Uh oh!

atztogo left a comment

Choose a reason for hiding this comment

Uh oh!

lan496 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

siteshwar commented Nov 20, 2025

Uh oh!

LecrisUT commented Nov 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

LecrisUT commented May 10, 2025 •

edited

Loading

codecov bot commented May 10, 2025 •

edited

Loading