-
Notifications
You must be signed in to change notification settings - Fork 7
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 assert
with if
...raise
#152
Comments
I think asserts are ok, especially for now. And we want to have a |
Yes, that's why I added "lo-pri" tag.
This is handled by python. In most cases we face, |
Some notes about exceptions, expected to be useful in this repo
Currently in |
In my opinion, it is ok to use asserts only for unrecoverable errors, i.e., errors for which the user is very unlikely to specify a catch implementation. For any other case we should use exceptions. |
I suggest we replace
assert
statements withif
...raise
in the library code (cirkit/*
), for the reasons:assert
does not work in all cases. It assumes__debug__ and condition
, which may be optimized out with different python config;AssertionError
does not make sense. We should use a more specific exception for each case.In unit tests we can keep
assert
as the above is not true. (1. Tests should be for debugging; 2.AssertionError
indicates normal exit of library code with the result unexcepted.)For benchmark and examples, where we use the library, we can keep
assert
because it's simpler, and we don't need the same level of robustness as the library itself.Is it better to use the format of
if not ...
? In this way it's more obvious what predicate should hold true.The text was updated successfully, but these errors were encountered: