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 #39: Add compatibility with >=pillow-7.0.0 #54
Conversation
Commit message should probably start with "Fix #39: ..." Also, does it still work with older pillow, or is this a new minimum requirement? |
At least, with version 6.x is working. I can't test version 5 and older. But If I understand the docs correctly, it should be the case. |
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.
That patch does not fly as it breaks nml for older versions of PIL / pillow:
nmlc ERROR: nmlc: An internal error has occurred:
nmlc-version: 2019-10-23-g909c09fd
Error: (AttributeError) "module 'PIL.Image' has no attribute 'version'".
Command: ['/home/ingo/bin/nmlc', '--version']
Location: File "/home/ingo/ottd/grfdev/nml/nml/version_info.py", line 142, in get_lib_versions
PIL: 1.1.7
PLY: 3.9
Thanks. But |
@planetmaker Could you try latest variant? |
Changed PILLOW_VERSION to PIL.__version__, as it got removed with >=pillow-7.0.0. In order not to break compatibility with older versions, PILLOW_VERSION will be used as a fallback. Closes: #39 Signed-off-by: Conrad Kostecki <conrad@kostecki.com>
It's PILLOW_VERSION (you are missing one L). That fixed it works for me. |
already fixed ;-) |
But the README says the minimum supported pillow version is 5.2, so why is this relevant? It will (I expect) then just break elsewhere? Or are older versions actually supported after all? |
PIL =! Pillow. Version Pillow 5.2.0 also reports |
Ah, of course, I misread the comment then. @planetmaker, what pillow version were you using when you posted:
Looking more closely, it seems this PR now uses |
@matthijskooijman I guess, it depends on the specific version, which he has. But currently, as far I understand the code, both, |
AFAICS, Lines 140 to 154 in abb7a3a
Looking at the code gain, I also wonder if the fallback to |
FWICS, you seem to be right, that |
Previously, this tried various combinations of version variables. However, some of these are deprecated by Pillow and the 6.0.0 release notes recommend simply using `PIL.__version` instead. This attribute is available since Pillow 3.4.0, so there is no need for any compatibility fallbacks. This relates to OpenTTD#29, OpenTTD#39 and OpenTTD#54.
Previously, this tried various combinations of version variables. However, some of these are deprecated by Pillow and the 6.0.0 release notes recommend simply using `PIL.__version` instead. This attribute is available since Pillow 3.4.0, so there is no need for any compatibility fallbacks. This relates to OpenTTD#29, OpenTTD#39 and OpenTTD#54.
Previously, this tried various combinations of version variables. However, some of these are deprecated by Pillow and the 6.0.0 release notes recommend simply using `PIL.__version` instead. This attribute is available since Pillow 3.4.0, so there is no need for any compatibility fallbacks. This relates to #29, #39 and #54.
Changed PILLOW_VERSION to PIL.version,
as it got removed with >=pillow-7.0.0.
In order not to break compatibility with older versions,
PILLOW_VERSION will be used as a fallback.
Closes: #39
Signed-off-by: Conrad Kostecki conrad@kostecki.com