-
Notifications
You must be signed in to change notification settings - Fork 871
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
Fix S101
, replace all assert
in code base (except for tests)
#4017
Fix S101
, replace all assert
in code base (except for tests)
#4017
Conversation
S101
, replace all assert
in code baseS101
, replace all assert
in code base (except for tests)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks a lot for all this work @DanielYang59! 🙏
will improve user experience a lot to get helpful error messages rather than a generic AssertionError
. to make the messages more informative, i think it would be great to include the offending value/type in all (Value|Type)Errors
(see 135f69e for a few examples)
No problem. Thanks for reviewing!
That's a very sensible suggestion, I would implement this later :) |
Manually trace this link 135f69e to the discussion on f-string (sometimes comment doesn't show up in the PR for reasons I don't know 😄 ), in case I want to get back to this thread. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
excellent work! 🙏
@@ -120,7 +120,8 @@ def __init__(self, wavelength="CuKa", symprec: float = 0, debye_waller_factors=N | |||
self.radiation = wavelength | |||
self.wavelength = WAVELENGTHS[wavelength] | |||
else: | |||
raise TypeError(f"{type(wavelength)=} must be either float, int or str") | |||
wavelength_type = type(wavelength).__name__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, thanks a lot for fixing all those (type(var).__name__
for name directly instead of <class 'xxx'>
). I totally forgot about them.
Summary
S101
, replace allassert
in code basePlease review all changes carefully to make sure the assert condition is correctly reversed (there're just too many changes and I tried not to make mistakes).