Skip to content

Conversation

@jorenham
Copy link

This is one of the additional checks that are required by repo-review (MY106): https://learn.scientific-python.org/development/guides/style/#MY106
IMO 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


try:
from . import BmpImagePlugin
from . import BmpImagePlugin as BmpImagePlugin
Copy link
Author

@jorenham jorenham Jan 27, 2026

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"):
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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.

Copy link
Member

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

Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants