-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Fix size calculation of Image.thumbnail() #4404
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this happened due to double rounding in both branches.
These latest commits have a fix related to the issue previously discussed. Rounding up the size contradicts the documentation which states:
So now when you request a thumbnail with a maximum size of As far as I can tell passing float values to size is deprecated, so that commit can be reverted when the switch is made. |
5f0d846
to
7d3c9b0
Compare
7d3c9b0
to
64c08f4
Compare
I have an interesting question. Let's say we have an image But the thing is, |
I'd like to point out that the current implementation had the same issue. It happens because the rounding is indiscriminate towards the aspect ratio. I've added a commit that fixes the issue, improvements are welcome. |
Thanks! |
The current size calculation is fine if only one axis is smaller or if the image is wide, but it trips up when both axes are smaller and the image is tall. Here's an example:
Someone made a pull request that fixed this in 2016, but it looks like he closed it shortly after due to a CI failure.