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 general asserts with specific exceptions #184

Open
dmdunla opened this issue Jul 7, 2023 · 1 comment
Open

Replace general asserts with specific exceptions #184

dmdunla opened this issue Jul 7, 2023 · 1 comment
Labels
enhancement New feature or request
Milestone

Comments

@dmdunla
Copy link
Collaborator

dmdunla commented Jul 7, 2023

Suggestion: Replace general asserts with specific exceptions based on context.

We currently use a lot of general asserts in the following pattern:

if (<condition>):
    assert False, "<str>"

Using specific exceptions -- e.g., ValueError, IndexError, etc. -- could be more helpful to users.

@dmdunla dmdunla added the enhancement New feature or request label Jul 7, 2023
@dmdunla dmdunla added this to the v2.0 milestone Jul 7, 2023
@ntjohnson1
Copy link
Collaborator

I think there is some nuance to the asserts. I think specific exceptions are more helpful for things that couldn't POSSIBLY be correct. However, in this PR I used an assert intentionally so if someone wanted to load larger data and didn't care about the guardrails they could using python -O to simply ignore the check entirely. I think that may be a cleaner logical separation. What should always halt at exception, and what are nice to have checks that we want to optimize away when running larger problems.

@dmdunla dmdunla modified the milestones: v2.0.0, v2.1.0 Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants