-
Notifications
You must be signed in to change notification settings - Fork 8
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
gh-248: fix all remaining possible ruff # noqa
statements
#394
Conversation
# TODO(ntessore): missing spin-1 pixel window function here # noqa: FIX002 | ||
# https://github.com/glass-dev/glass/issues/243 |
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.
Have just expanded the issue so we can remove FIX002
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, @paddyroddy! Looks good to me 🙌🏼
glass/points.py
Outdated
assert np.sum(n[stop:]) == 0 # noqa: S101 | ||
if np.sum(n[stop:]) != 0: | ||
msg = "The number of pixels sampled does not match the expected count." | ||
raise ValueError(msg) |
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.
This is also something that can only happen when something is intrinsically broken.
Is S101
really a helpful rule if it flags instances where an assert is definitely the right tool for the job?
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.
So it's if someone runs python -O
in which case asserts
will be removed https://docs.astral.sh/ruff/rules/assert
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 can revert though
Have addressed @ntessore |
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.
LGTM, excellent work!
Closes #248. Fixing all possible
noqa
statements. Leaving those that can't really be fixed, like too many function inputs. I expect some push back on these, so let's iterate.