Skip to content

Use more standard __version__ rather than PILLOW_VERSION #33

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

Closed
wants to merge 3 commits into from

Conversation

hugovk
Copy link
Contributor

@hugovk hugovk commented Apr 18, 2018

@bgilbert
Copy link
Member

Hi, thanks for the PR. __version__ is only available in Pillow ≥ 3.4.0. As the comment says on the preceding line, PILLOW_VERSION is available in Pillow ≥ 2.1.0, so this change appears to make the Pillow detection strictly worse. What problem is this PR intended to solve?

@hugovk
Copy link
Contributor Author

hugovk commented Apr 19, 2018

We're considering removing PILLOW_VERSION from Pillow in a future major release, as it was a hack we should probably fix.

How about requiring Pillow ≥ 3.4.0 in setup.py?

install_requires=[
'Pillow',
],

Pillow 3.4.0 was released in October 2016. Since 2.1.0, there have been security fixes in 2.3.1, 2.3.2, 2.5.2, 2.5.3, 2.7.0, 3.1.1, 3.1.2 and 3.3.2, so it might be wise to require something newer.

https://www.cvedetails.com/product/27460/Python-Pillow.html


Alternatively, how about a try/except for __version/PILLOW_VERSION?

@bgilbert
Copy link
Member

Thanks for the additional info. We don't control what version of PIL or Pillow the user has on their system, though. For example, Ubuntu 14.04 LTS ships Pillow 2.3.0 (hopefully with backported security fixes).

What about

have_optimizations and not (hasattr(Image, 'PILLOW_VERSION') or hasattr(Image, '__version__'))

with an appropriate comment?

@hugovk
Copy link
Contributor Author

hugovk commented Apr 19, 2018

That looks good. I'll update it.

By the way, install_requires means you would control what version the user has: https://stackoverflow.com/a/8161816/724176

install_requires=[ 
     'Pillow >= 3.4', 
 ], 

Then when you pip install openslide-python it would install at least that version of Pillow. The "skips Pillow < 2.1.0" could be removed entirely.

@hugovk
Copy link
Contributor Author

hugovk commented Aug 8, 2019

@bgilbert Hi, any thoughts on this? Thanks!

@hugovk
Copy link
Contributor Author

hugovk commented Jan 3, 2020

In Pillow 7.0.0, just released:

PILLOW_VERSION has been removed. Use __version__ instead.

https://pillow.readthedocs.io/en/stable/deprecations.html#pillow-version-constant

@hugovk
Copy link
Contributor Author

hugovk commented Sep 13, 2020

Update:

It [PILLOW_VERSION] was initially removed in Pillow 7.0.0, but brought back in 7.1.0 to give projects more time to upgrade.

https://pillow.readthedocs.io/en/stable/deprecations.html#pillow-version-constant

It'll probably be removed next year.

Closing this PR because it's over two years old. It's still relevant so I can re-open if there's any interest in merging. Thanks!

@hugovk hugovk closed this Sep 13, 2020
@bgilbert
Copy link
Member

Let's keep this open for now. Since the constant has been deprecated, we'll need to change this eventually. Once we revisit which Linux distros still make sense to support, we'll have a better idea whether we still need the compat support or can just drop it.

@bgilbert bgilbert reopened this Sep 13, 2020
@bgilbert bgilbert closed this in 9e9f960 Sep 13, 2020
@bgilbert
Copy link
Member

9e9f960 updates the check to require Pillow ≥ 6.2.0 for #31, and I've switched to __version__ at the same time. So this should be fixed now.

Thanks very much for the PR and for your patience.

@hugovk
Copy link
Contributor Author

hugovk commented Sep 13, 2020

You're welcome, and thanks!

@hugovk hugovk deleted the patch-1 branch September 13, 2020 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants