Skip to content

Use the correct error types#258

Merged
james-d-mitchell merged 9 commits into
libsemigroups:v1from
Joseph-Edwards:check-correct-errors
Jun 9, 2025
Merged

Use the correct error types#258
james-d-mitchell merged 9 commits into
libsemigroups:v1from
Joseph-Edwards:check-correct-errors

Conversation

@Joseph-Edwards

Copy link
Copy Markdown
Collaborator

This PR ensures the correct errors are being thrown, and that we document the errors that actually get thrown. It also updates how we register our custom LibsemigroupsError to move away from some deprecated functions.

There are a few places where we arguably throw the wrong error (for example, we throw a LibsemigroupsError rather than a ValueError when trying to add a generator to a FroidurePin with the wrong degree). We could fix this, but this seems like more hassle than it's worth.

Comment thread src/errors.cpp Outdated

@james-d-mitchell james-d-mitchell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Obviously the CI test should pass, and I think we should stick to the paradigm that all exceptions raised in libsemigroups C++ code are translated to LibsemigroupsErrors, which it's a little unclear to me if that's what you've implemented. I'll add a couple more specific comments about that.

Comment thread tests/test_words.py
@Joseph-Edwards

Copy link
Copy Markdown
Collaborator Author

Once libsemigroups/libsemigroups#736 is merged, this PR should be good to go from my perspective.

@Joseph-Edwards Joseph-Edwards marked this pull request as draft June 6, 2025 16:19
@Joseph-Edwards

Copy link
Copy Markdown
Collaborator Author

Upon further consideration, this PR seems like a good place to include the "describe the _libsemigroups_pybind11 module" changes, so I'll do that now too.

@Joseph-Edwards Joseph-Edwards marked this pull request as ready for review June 7, 2025 14:40
@Joseph-Edwards Joseph-Edwards force-pushed the check-correct-errors branch from 190036e to 46c6de4 Compare June 9, 2025 12:39
@james-d-mitchell james-d-mitchell merged commit 349b4ae into libsemigroups:v1 Jun 9, 2025
11 checks passed
@james-d-mitchell

Copy link
Copy Markdown
Member

Excellent job @Joseph-Edwards, couldn’t have done any better on the description of the structure of the package!

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.

2 participants