Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace production code asserts with if-based checks and exception raises #70

Merged
merged 1 commit into from
Dec 16, 2024

Conversation

d33bs
Copy link
Collaborator

@d33bs d33bs commented Dec 16, 2024

This PR replaces some production code asserts with if-based checks and exception raises (when appropriate) to simulate the use of assertions without the related risks. It's considered a best practice to avoid the use of asserts in production code as they may be ignored during processing by using the python -O flag (optimization mode) and could be replaced with more specific exception types + messages.

Thanks for any feedback!

Closes #64

@d33bs d33bs marked this pull request as ready for review December 16, 2024 16:56
Copy link
Collaborator

@miltondp miltondp left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@d33bs
Copy link
Collaborator Author

d33bs commented Dec 16, 2024

Thanks @miltondp and @falquaddoomi ! Merging this in.

@d33bs d33bs merged commit f2ac788 into manubot:main Dec 16, 2024
7 checks passed
@d33bs d33bs deleted the remove-production-asserts branch December 16, 2024 22:04
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.

Replace assert statements in production code
3 participants