Pillow
Pillow copied to clipboard
Misleading deprecations documentation for getsize
The documentation suggests that this old code
width, height = font.getsize("Hello world")
should be replaced by
left, top, right, bottom = font.getbbox("Hello world")
width, height = right - left, bottom - top
A 1:1 replacement would require different new code
left, top, right, bottom = font.getbbox("Hello world")
width, height = right - left, bottom
The documentation should mention that and maybe link to this comment.
I think the argument would be that the old methods were incorrect, which is why they were deprecated, and that we're encouraging users to use the correct new methods, with the new correct results.
Explaining to users how to keep things as they were seems to work against that purpose.
Yes, sure, but a short comment which explains that the old method was incorrect would not hurt, but help. There might be old code which circumvented the deficits of the old method, and there is code which requires additional changes, not simply the suggested replacement.
https://github.com/Belval/TextRecognitionDataGenerator/pull/323 for example changes the code as suggested in the deprecations documentation, but that breaks the unit test of that software.
I've created #7806. See what you think.
I think that the documentation should include the transition code, as provided by OP. So old code can easily be converted without changing the output. And as it's not easy to understand that the old "height" is yet obtened by the new "bottom", the transition code added to the documentation can be very heplful. Or, if you would not add the transition code, perhaps a picture showing what is really the "bottom" in the font.getbbox("Hello world") can also help. Thanks. Big up to @stweil for his transition code in this issue. Very very very helpful.
The OP spoke about this change breaking unit tests in https://github.com/Belval/TextRecognitionDataGenerator/pull/323. I don't personally think that necessitates simulating Pillow's old behaviour - I think updating the unit test is the more thought out solution.
In the case of https://github.com/uvipen/ASCII-generator/issues/17, it looks like the code is drawing characters in order to evaluate the brightness. Pillow's old behaviour would have meant that some blank space was included above the character. I don't think you want to consider that in that calculation.
So I'm still not seeing why we should be encouraging users to continue doing things in a less-than-ideal way. I think this additional documentation should make it clear enough for users to figure it out how to do so on their own if they really want.
As for a picture, sure, I've added that to the PR.
https://pillow--7806.org.readthedocs.build/en/7806/deprecations.html#font-size-and-offset-methods