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

refactor images.py to use PIL ImageOps for much simpler code #1703

Merged
merged 1 commit into from
Oct 25, 2023

Conversation

Bomme
Copy link
Contributor

@Bomme Bomme commented Oct 23, 2023

I was following the tutorial in
https://pillow.readthedocs.io/en/stable/handbook/tutorial.html#relative-resizing

I started refactoring since I wanted to remove all calls to old_div but found it was easier just to rewrite large portions relying on PIL library code

I was following the tutorial in
https://pillow.readthedocs.io/en/stable/handbook/tutorial.html#relative-resizing

I started refactoring since I wanted to remove all calls to old_div but
found it was easier just to rewrite large portions relying on library
code
@Bomme Bomme requested review from alastair and ffont October 23, 2023 22:05
@Bomme
Copy link
Contributor Author

Bomme commented Oct 24, 2023

Differences between old and new code

Original image (97x128)

97_128shape

Note: I added a one pixel border in black for demonstration purposes

size old new
50x50 50_50shape_old_border 50_50shape_new_border
100x100 100_100shape_old_border 100_100shape_new_border
150x150 150_150shape_old_border 150_150shape_new_border

Copy link
Member

@ffont ffont left a comment

Choose a reason for hiding this comment

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

That looks great, thanks for the contribution!
The white added space will not look nice for some avatars (specially in dark mode), but then again users can fix that by uploading a properly squared avatar so this is fine.

@ffont ffont merged commit a1182ae into master Oct 25, 2023
1 check passed
@ffont ffont deleted the refactor_images branch October 25, 2023 09:13
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