Bug 243665 - graphics/py-pillow: Update to 7.0.0
Summary: graphics/py-pillow: Update to 7.0.0
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Kai Knoblich
URL: https://pillow.readthedocs.io/en/stab...
Keywords:
Depends on:
Blocks:
 
Reported: 2020-01-28 09:31 UTC by Kai Knoblich
Modified: 2020-02-16 09:33 UTC (History)
3 users (show)

See Also:
koobs: maintainer-feedback+


Attachments
py-pillow-7.0.0.patch (2.77 KB, patch)
2020-01-28 09:31 UTC, Kai Knoblich
kai: maintainer-approval? (koobs)
Details | Diff
py-pillow-7.0.0-bump-consumers.patch (66.15 KB, patch)
2020-01-28 09:42 UTC, Kai Knoblich
no flags Details | Diff
py-pillow-7.0.0-bump-consumers-v2.patch (65.20 KB, patch)
2020-02-13 13:04 UTC, Kai Knoblich
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kai Knoblich freebsd_committer 2020-01-28 09:31:09 UTC
Created attachment 211124 [details]
py-pillow-7.0.0.patch

Attached is sunpoet's patch from bug #243336 with additional CONFLICTS_INSTALL entries.

Before applying the patch one needs to do "svn cp graphics/py-pillow graphics/py-pillow6".
Comment 1 Kai Knoblich freebsd_committer 2020-01-28 09:41:09 UTC
Results of "make test" for each Python flavor:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

With py37, py38:

> ==== 1298 passed, 129 skipped in 32.14 seconds ====
> ==== 1298 passed, 129 skipped in 31.40 seconds ====

With py35:

> ==== 1296 passed, 131 skipped in 40.62 seconds ====

With py36:

> ==== 1297 passed, 130 skipped in 33.41 seconds ====

Version requirements in setup.py, etc.:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Except two ports no consumer has pinned requirements for graphics/py-pillow. The two exceptions are:

- net-mgmt/netbox: OK, patched out in Makefile
- www/py-wagtail: OK, defined in Makefile and setup.py as "<7.0".


More thorough QA results regarding the backwards incompatible changes that were introduced in the 7.0.0 release (see also the release notes in the URL field for further detail):


Removal of the PILLOW_VERSION constant:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- graphics/impressive: OK, still uses Python 2.7 only and already has compatible code to cope with different Pillow versions.
- graphics/py-blockdiag: OK, still uses Python 2.7 only. No compatible code for newer Pillow versions.
- graphics/py-mcomix: OK, still uses Python 2.7 only. No compatible code for newer Pillow versions.
- graphics/sk1: OK, still uses Python 2.7 only. No compatible code for newer Pillow versions.
- print/hplib: OK, already patched out in r503333 as preparation for the 6.0 release of graphics/py-pillow.
- x11/xpra: OK, still uses Python 2.7 only and already has compatible code to cope with different Pillow versions.

Conclusion: OK, as all those consumers (except print/hplib maybe, as it already runs with Python 3) will get graphics/py-pillow6 as a dependency and should work as usual.

Removal of the *ImagePlugin.__version__ constants:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- OK, no port makes use of these constants.


Removal of PyQt4 and PySide code:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- OK, not relevant as Qt4 and PySide are already gone from the Ports tree.


Removal of setting the image size for TIFF images by "im.size(x, y):
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- graphics/py-imageio: OK, uses "im.size = ( ... )" only in documentation context in "imageio/plugins/pillow_info.py".
- graphics/py-imageio24: OK, uses "im.size = ( ... )" only in documentation context in "imageio/plugins/pillow_info.py".
- net-p2p/deluge-cli: OK, uses Python 2.7 only and "im.size = ( ... )" is used for ICO files. In this context using "im.size = ( ... )" is allowed as it selects a subimage.


Changed return value of "Image.draft()":
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- graphics/py-mcomix: OK, uses Python 2.7 only.
- graphics/py-sane: OK, uses Python 2.7 only.

Conclusion: OK, as all those consumers will get graphics/py-pillow6 as a dependency and should work as usual.


Default resampling filter:
~~~~~~~~~~~~~~~~~~~~~~~~~~
The default resampling filter was changed from "IMAGE.NEAREST" to "IMAGE.BICUBIC" for the "fit()", "pad()", "resize()" and "scale()" functions. Those functions are used by several consumers for image processing/manipulation.

Conclusion: Should be OK, but we should keep in mind that the changes of the above mentioned functions will have definitely a small impact on the Pillow consumers. A bit more CPU cycles than usual are then required for image processing but yields in better image results/quality. 

If there are problems once Pillow has been updated to 7.0.0 we can still assign the problematic consumers (if any) to Pillow 6.2.2 and bump PORTREVISION.
Comment 2 Kai Knoblich freebsd_committer 2020-01-28 09:42:50 UTC
Created attachment 211125 [details]
py-pillow-7.0.0-bump-consumers.patch

Attached a prelimary patch that changes the *_DEPENDS to "graphics/py-pillow6" and bumps the PORTREVISION of all consumers that can still be built for Python 2.7 or later.

Exceptions are ports, that have "USES=python:3.[0-9]+" and no conflicting dependencies that could break "bulk -a":

- astro/py-astLib
- audio/lollypop
- games/openage
- games/unknown-horizons
- graphics/py-cairosvg
- graphics/py-imageio
- graphics/py-pyocr
- graphics/variety
- math/py-plastex
- misc/py-tflearn
- net-im/py-matrix-synapse
- net-mgmt/netbox
- science/gramps
- www/py-weboob
- www/xist
Comment 3 Kai Knoblich freebsd_committer 2020-01-28 09:45:04 UTC
Technically a few more ports could make of use of Pillow 7.0.0 if the Python 2.7 build is deactivated for them, e.g. by setting "USES=python:3.5+", to avoid "bulk -a" breakages. It would also save a few PORTREVISION bumps and would be a few more steps towards the deorbit of Python 2.7.

I left those ports out from the initial patch for the sake of brevity:

Ports have no consumers that depend upon them and no conflicting dependencies:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- devel/py-pydenticon
- games/py-hypatia_engine
- graphics/py-cartopy
- graphics/py-photocollage
- graphics/py-pypillowfight
- misc/py-gluoncv
- security/vinetto (unmaintained)
- www/py-instabot


Ports that doesn't have "USE_PYTHON=distutils" but can be also built for any Python version:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- cad/k40-whisperer
- comms/apitran
- math/asymptote
- misc/cs
- misc/mmdnn

Ports that have "USE_PYTHON=distutils noflavors":
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- games/pysolfc (I plan to drop the Python 2.7 support as part of this issue.)


If we want to go that route with dropping the Python 2.7 support for those ports we should do it before landing Pillow 7.0.0, IMHO.
Comment 4 Kubilay Kocak freebsd_committer freebsd_triage 2020-01-28 10:52:51 UTC
Thank you for this Kai, greatly appreciated, especially the extensive QA

Pillow 7 update looks good.

Couple of notes:

Regarding www/py-wagtail: OK, defined in Makefile and setup.py as "<7.0"...

We should probably its *_DEPENDS to ${PYTHON_PKGNAMEPREFIX}pillow<7.0:graphics-py-pillow6

More broadly (with the above as an example), as part of the QA, if we come across any ports that have depends either inconsistent or incomplete (especially ">X & <Y" cases) with their setup.py's, we should fix those and make them match.

Those changes are:

Approved by: portmgr (blanket(s): Fix dependencies, framework/python compliance)
Comment 5 Kai Knoblich freebsd_committer 2020-02-13 13:04:41 UTC
Created attachment 211612 [details]
py-pillow-7.0.0-bump-consumers-v2.patch

Attached is an updated patch for the consumers of graphics/py-pillow.

(In reply to Kubilay Kocak from comment #4)

We've already discussed the situation about www/py-wagtail on IRC some days ago. The dependency for graphics/py-pillow with "< 7.0" is declared correctly in the port's Makefile and in the setup.py thus no particular action is required.

I also assume that I've your approval to land Pillow 7.0.0. ;-)
Comment 6 Kubilay Kocak freebsd_committer freebsd_triage 2020-02-14 01:17:41 UTC
(In reply to Kai Knoblich from comment #5)

You have the helm Kai, thank you
Comment 7 Sunpoet Po-Chuan Hsieh freebsd_committer 2020-02-14 09:59:46 UTC
(In reply to Kai Knoblich from comment #5)

Please hold on. I assume we'll have pillow 7.0.0 landed with dependent ports having conditional dependency on pillow/pillow6. But the patches just moves all dependent ports to pillow6.
Comment 8 Kai Knoblich freebsd_committer 2020-02-14 18:43:53 UTC
(In reply to Sunpoet Po-Chuan Hsieh from comment #7)

Not all ports are assigned to graphics/py-pillow6 as noted in comment #2. 

I assume you mean something like this for consumers that can also be built for Python 2.7:

> .if ${PYTHON_REL} < 3500
> RUN_DEPENDS+=   ${PYTHON_PKGNAMEPREFIX}pillow6>=0:devel/py-pillow6@${PY_FLAVOR}
> .else
> RUN_DEPENDS+=   ${PYTHON_PKGNAMEPREFIX}pillow>=0:devel/py-pillow@${PY_FLAVOR}
> .endif

I initially included the above in my considerations and I'm not entirely averse to it either, because more ports can be used with Pillow 7.0.0 then.

But I went away from that idea for now as it also makes the whole scenario more complex, IMHO. (I'd start a review on Phabricator then to get a better overall picture).

I'd just like to know if there are good reasons to extend the grace period of Python 2.7 by adding the conditionals from above?
Comment 9 Sunpoet Po-Chuan Hsieh freebsd_committer 2020-02-14 20:30:57 UTC
(In reply to Kai Knoblich from comment #8)

> Not all ports are assigned to graphics/py-pillow6 as noted in comment #2. 

Oh, I overlooked that.

I think pillow has enough consumers (almost 100) to have PY_PILLOW in Mk/Uses/python.mk. That's also what I planned for math/py-scipy update.

> I'd just like to know if there are good reasons to extend the grace period of Python 2.7 by adding the conditionals from above?

According to EXPIRATION_DATE of python27, it has 1-year grace period. I still keep python 2.7 support temporarily.

In other words, if a port drops python 2.7 support, could we simply restrict all its dependent ports to use python 3 (if they all work fine)?
Comment 10 Kubilay Kocak freebsd_committer freebsd_triage 2020-02-15 00:03:19 UTC
(In reply to Sunpoet Po-Chuan Hsieh from comment #9)

I'd like to see less dependencies in python.mk than more. Every Python package that declares a dependency that includes a <version-spec> cant be accurately or correctly described using a shared dependency line

It doesn't even matter whether or not the Python package declares the <version-spec> correctly or not. Whatever is specified in setup.py *must* be 'exactly' satisfied, otherwise it results in a runtime dependency error (even when the dependency is installed).
Comment 11 Kai Knoblich freebsd_committer 2020-02-16 09:33:38 UTC
(In reply to Sunpoet Po-Chuan Hsieh from comment #9)
(In reply to Kubilay Kocak from comment #10)

Ok, I created a review at Phabricator that contains conditional statements to use either Pillow 7 or Pillow 6 for consumers that can be built with Python 2 or newer.