-
Notifications
You must be signed in to change notification settings - Fork 146
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
Reraise exceptions when debugging #2167
Reraise exceptions when debugging #2167
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.
re-raising looks good to me
evap/evaluation/models.py
Outdated
else: | ||
logger.error(warning_message) | ||
level = logging.WARNING | ||
messages.warning(request, _(message)) |
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.
Does _(message)
even work with the user data already formatted into message
? Don't we usually do _("look at this data: {}").format(the_data)
?
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.
reverted the 'simplification' and fixed the issue. Only did it in the first place to keep under ruff's too-many-branches
. This is now only covered with pylint
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.
Do you know why ruff raised a pylint-based lint while pylint itself did not raise it? If the two check the same, we should probably prefer ruff for speed, right? (not a blocker for this PR)
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.
There was disagreement between pylint and ruff about how many branches there really are in the function. I think ruff counted one more than pylint, so the pylint check doesn't trigger while the ruff check does. Don't know if it's a ruff bug or if ruff thinks it's a pylint bug.
In general, I'd agree with @niklasmohrin to go with ruff for speed.
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.
Okay, maybe lets add this as a comment so we can revisit this later
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.
Ok, I will then enable ruff only and increase the limit by 1 taking the except
-counting into account
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 well no. ruff has a bug there. It does not count branches inside with
statements... astral-sh/ruff#11313
4971beb
to
d934ff7
Compare
This was raised during debugging #2136, concerning
update_evaluations
. There, errors due to typeguard were silenced in the tests because of the raw except clause leading to weird test failures.This adds checks to reraise the exceptions when debugging.