enable the mypy "truthy-bool" error code#9409
enable the mypy "truthy-bool" error code#9409jorenham wants to merge 2 commits intopython-pillow:mainfrom
Conversation
|
|
||
| try: | ||
| from . import BmpImagePlugin | ||
| from . import BmpImagePlugin as BmpImagePlugin |
There was a problem hiding this comment.
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.
at this point image cannot be None anymore
|
|
||
| reloaded = Image.fromarrow(arr, mode, img.size) | ||
|
|
||
| assert reloaded |
There was a problem hiding this comment.
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.
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.
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.
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