Skip to content
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

using with clause to open img with pil to guarantee it is closed #1174

Merged
merged 1 commit into from
Apr 7, 2024

Conversation

serengil
Copy link
Owner

@serengil serengil commented Apr 7, 2024

Tickets

What has been done

With this PR, we added with close while opening file with PIL to guarantee it is closed after processed.

How to test

make lint && make test

# similar to find functionality, we are just considering these extensions
# content type is safer option than file extension
if file_type not in ["jpeg", "png"]:
raise ValueError(f"input image can be jpg or png, but it is {file_type}")
with Image.open(io.BytesIO(decoded_bytes)) as img:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually not needed as in this case we're not opening a file handle: all data is already in memory (decoded_bytes). Hence the auto closure in a with block has no effect.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity: why are you proxying decoded_bytes through io.BytesIO
The signature of Image.open allows to pass directly the decoded_bytes bytes object

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean Image.open(decoded_bytes) performs same? TBH, ChatGPT showed me that usage.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For with block, agree it has no effect but still i believe it is good practice to confirm opened things closed.

Copy link
Contributor

@AndreaLanfranchi AndreaLanfranchi Apr 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean Image.open(decoded_bytes) performs same? TBH, ChatGPT showed me that usage.

Never ever trust ChatGPT blindly ... I beg you.
It can give you some hints but never let AI write code for you unless you have rock solid certainty it is correct, reliable and best practice. Copy pasting from ChatGPT does not tell you why some actions, statements or algos are implemented. As a developer you have to be always in the position to explain the why(s) and own the code logic.
Please.

@serengil serengil merged commit 42ee298 into master Apr 7, 2024
4 checks passed
@serengil serengil deleted the feat-task-0704-determine-file-type-with-pil branch April 7, 2024 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants