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
chore: resolve Ruff warnings A003, B023, D103 and PLR0911 #684
Conversation
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.
Very good, appreciate this one, thank you :-)
Co-authored-by: Johannes Nussbaum <39048939+jnussbaum@users.noreply.github.com>
@@ -136,38 +137,43 @@ def find_date_in_string(string: str) -> Optional[str]: | |||
# sanitize input, just in case that the method was called on an empty or N/A cell | |||
if not check_notna(string): | |||
return None | |||
try: | |||
return _find_date_in_string_throwing(string) |
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.
I don't particularly like this pattern. In fact, no one of the called functions should raise an error, they should just return None if they don't succeed. Then, we don't need a function with this strange name.
(I know that it was me who introduced the ValueErrors, but now I don't like them anymore)
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.
I am with you on that, but I didn't want to make bigger changes in a simple linting PR. Feel free to adjust that in a separate PR and I will happily approve it :)
No description provided.