Skip to content
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

Broken operation with pillow version 7.0 #28

Closed
pp-mo opened this issue Jan 9, 2020 · 1 comment
Closed

Broken operation with pillow version 7.0 #28

pp-mo opened this issue Jan 9, 2020 · 1 comment

Comments

@pp-mo
Copy link
Member

pp-mo commented Jan 9, 2020

Discovered in course of Iris#3628 that Iris graphics tests have just broken.

After some investigation, determined that this is due to changed results from key pillow (aka PIL) operations that imagehash.phash() relies on, since pillow v7.0.0 came out.

  • the whole usage of iris graphics tests (IrisTest.check_graphic), the iris/tests/idiff.py command, and this repo currently relies on the idea that an image produces a unique stable hash value, determined by 'imagehash.phash(image)', and we use that hash value (in hex ) as the name for the image file stored in test-iris-imagehash
  • the hashes of all our stored image files have in fact remained stable since we first developed test-iris-imagehash.
  • this relationship is tested by test-iris-imagehash/run_test.py : It checks that all the files compute a hash equal to their filename. Until fixed, this will break all PR tests from now on

On reflection, fixing this is all a bit tricky : There is no guarantee or expectation in pillow (aka PIL) that the results of operations would be exactly the same between different package versions. In fact, even imagehash itself does not make such a claim : it provides provide toleranced image compare by calculating hashes that can be differenced, but this does not mean that the hash values themselves are necessarily portable from one version to another.
We already had one such problem, where hash numbers changes in imagehash version 4, as fixed in #14. Arguably, we have just been "lucky" since then (!) More recently, imagehash#32 raises a similar problem with possible pillow changes (that presumably worked out ok).

Temporarily fixed in Iris by pinning : SciTools/iris#3630


A "proper" fix would involve changes both here and in Iris.
One possible way ...

  • associate hashes with specific dependency versions (imagehash, pillow, possibly scipy?)
  • decouple storage filenames from the hash info (or possibly use softlinks as was done in Add imagehash v4 links #14)
  • make sure Iris only uses hashes appropriate to the current installed library(s) when calculating compare distances
    This assumes we need to retain function with old versions of dependencies, and implies we must know hashes for all files at multiple versions.

Another way is only to support a latest version (fixed or pinned), and insist only that is useable.
This means when problems occur we would rename all the image files, and change Iris imagerepo.json. Tests will then no longer work with older versions of pillow (+poss others).

@rcomer
Copy link
Member

rcomer commented Aug 1, 2022

I guess we can close this, thanks to @wjbenfold in SciTools/iris#4759 and SciTools/iris#4826.

@rcomer rcomer closed this as completed Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants