-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
enable the mypy "truthy-bool" error code #9409
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
base: main
Are you sure you want to change the base?
Conversation
|
|
||
| try: | ||
| from . import BmpImagePlugin | ||
| from . import BmpImagePlugin as BmpImagePlugin |
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 might look a bit weird (unless you're used to writing type stubs), but it avoids a # noqa. I wouldn't mind using # noqa here instead if that's preferred
| pass | ||
| else: | ||
| if image and image.mode in ("1", "L"): | ||
| if image.mode in ("1", "L"): |
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.
at this point image cannot be None anymore
|
|
||
| reloaded = Image.fromarrow(arr, mode, img.size) | ||
|
|
||
| assert reloaded |
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 could also change this to an assert isinstance or something, but I figured that'd also be kinda obvious.
| # Segfault test | ||
| app: QApplication | None = QApplication([]) | ||
| ex = Example() | ||
| ex = Example() # noqa: F841 |
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'm not sure what the idea here is, but it looked as if the assert was purely there to avoid this F841 error.
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.
Is it not preferable to avoid noqa where possible? My solution would have been assert ex is not None
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.
Personally I prefer being explicit about ignored errors. And assert statements aren't "free" (the runtime impact is probably negligible, but still).
And, if at some point, some change causes this so that ex could actually become None, then ruff will complain about an unused # noqa, which would prevent a potential bug.
This is one of the additional checks that are required by repo-review (
MY106): https://learn.scientific-python.org/development/guides/style/#MY106IMO it's more of a linting rule than a type-checking rule, but either way, I think it's a pretty decent one to enable. FWIW; In the projects I maintain (numpy, scipy-stubs, ...), I always enable it.
mypy docs: https://mypy.readthedocs.io/en/stable/error_code_list2.html#check-that-expression-is-not-implicitly-true-in-boolean-context-truthy-bool