Wait before we actually need to process the image for checking if Pillow can handle it. #72
Conversation
…low can handle it. This avoids loading the image needlessly.
patrickfournier
force-pushed
the
pillow-lazy-check
branch
2 times, most recently
from
2628392
to
41a5152
Compare
patrickfournier
force-pushed
the
pillow-lazy-check
branch
from
41a5152
to
90c3fc1
Compare
There was a problem hiding this comment.
@patrickfournier thank you, as always, for this. I think it would work (functionally) as is, but I've added some note that I think would improve it.
| i = Image.open(path) | ||
| except UnidentifiedImageError: | ||
| logger.warning( | ||
| "%s Source image %s is not supported by Pillow.", |
There was a problem hiding this comment.
When putting a filename like this in a log message, I like to wrap the name in quotes (single or double).
This is probably more for Windows, as Rich (the log processing library) doesn't consistently pick out Windows filenames.
| return None | ||
| except FileNotFoundError: | ||
| logger.warning( | ||
| "%s Source image %s not found.", |
There was a problem hiding this comment.
I would put this filename in quotes too (single or double, but match above).
|
|
||
| i = Image.open(image[0]) | ||
| i = try_open_image(image[0]) | ||
| if i is None: |
There was a problem hiding this comment.
It feels like it would be better (more Python-ic) to re-raise the exemptions in try_open_image(), and then catch them here.
Are there other cases (than raised exemptions) that i could come back as None?
| LOG_PREFIX, | ||
| path, | ||
| ) | ||
| return None |
There was a problem hiding this comment.
As I note below, it seems to me that it would be more Python-ic to re-raise the exemption, rather than returning a placeholder value like this.
Something like:
try:
# ...
except UnidentifiedImageError as e:
# log message
raise e
…recommendations Add Release.md Closes #72 # Conflicts: # pelican/plugins/image_process/image_process.py
This avoids loading the image needlessly.