enable the mypy "truthy-bool" error code by jorenham · Pull Request #9409 · python-pillow/Pillow

Conversation

jorenham


try:
from . import BmpImagePlugin
from . import BmpImagePlugin as BmpImagePlugin

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"):

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

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

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.

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

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.

Labels

2 participants

@jorenham @radarhere