search

conda-forge / pillow-feedstock

A conda-smithy repository for pillow.
BSD 3-Clause "New" or "Revised" License
2 stars 29 forks source link

pillow 4.2.1 regression with font.getsize on empty strings #34

Closed akrherz closed 7 years ago

akrherz commented 7 years ago

My apologies if this is an inappropriate bug to report here, but the release of 4.2.1 Pillow onto conda-forge has a regression that hit my code hard. The regression is logged here: python-pillow/Pillow#2614

  1. Is it appropriate to ask for the conda-forge version to be rolled back or
  2. make a PR with that patch included that bumps the conda-forge version or
  3. these types of conda-forge / bleeding-edge breakages are expected and just need to wait for next upstream release?

thank you for your time and making this available for conda.

ocefpaf commented 7 years ago

The best options are:

Is it appropriate to ask for the conda-forge version to be rolled back or

Not really b/c this is ultimately an upstream issue but you can always pin your workflow to the previous working version. This is usually the fastest b/c it depends only on things you can control (most of the times :smile: ).

make a PR with that patch included that bumps the conda-forge version or

That is a more involved solution but probably better. If you have the time please do so!

these types of conda-forge / bleeding-edge breakages are expected and just need to wait for next upstream release?

4.2.1 is an official release so I would not call "bleeding-edge breakages." Unfortunately even "stable" releases causes breakages though.

akrherz commented 7 years ago

@ocefpaf Thank you very much for the detailed response. I don't have enough skill to properly generate and test a PR for this. If you want to close this or just leave it open until upstream releases 4.2.2, that would be fine by me.

ocefpaf commented 7 years ago

Do you have a small image/script that reproduces the problem?

akrherz commented 7 years ago

@ocefpaf not really, the issue is already fixed upstream. The reproducer is simply to call font.getsize on an empty string.

ocefpaf commented 7 years ago

Got it. See #35

akrherz commented 7 years ago

oh wow, providing patches is a lot more automated than I thought. Thank you kindly for doing this.

ocefpaf commented 7 years ago

There is more there than only the patch. It is not a "clean" PR.

The "correct" way to do this would be to submit the patch, bump the build number, and add a test. I did not do the latter b/c it is tested upstream. And the PR contains a change in our pinning policies and a re-render.

TL;DR just the patch would be much easier :smile:

akrherz commented 7 years ago

Just to note that the newly released 4.3.0 Pillow still has an issue closely related to this one. see python-pillow/Pillow#2783

ocefpaf commented 7 years ago

Thanks for the heads up!

hugovk commented 7 years ago

The two issues: