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

Incosistent imread() function behavior #280

Closed
charlesreid1 opened this issue Nov 9, 2017 · 2 comments
Closed

Incosistent imread() function behavior #280

charlesreid1 opened this issue Nov 9, 2017 · 2 comments

Comments

@charlesreid1
Copy link
Contributor

charlesreid1 commented Nov 9, 2017

Hello,

In image_sequence.py there is a conditional import statement, in which an imread() function is imported from either scikit-image, matplotlib, or scipy. However, the behavior of each imread() function is different:

  • skimage.io.imread does not normalize pixel values, so they are integers from 0-255
  • matplotlib.pyplot.imread does normalize pixel values, so they are floats from 0-1
  • scipy.ndimage.imread is deprecated in favor of matplotlib.pyplot.imread (as printed in the output of calls to scipy's imread() function), but it uses integer pixel values 0-255

Also see this gist.

In issue #121 there was discussion of whether to make scikit-image a dependency, and it sounds like the consensus is no. In light of this, because we cannot count on scikit-image being installed on a user's machine, and because matplotlib is a more lightweight requirement (in keeping with the philosophy of keeping pims lightweight), it might be best to change lines 24-33 to simply be:

from matplotlib.pyplot import imread

This would affect the output of any script that created an ImageSequence object and had scikit-image installed (for example, the walkthrough notebooks of trackpy, see issues soft-matter/trackpy-examples#43 and soft-matter/trackpy#455) so it is a potentially breaking change if users are counting on pixel values to be between 0-255. But I believe consistent, predictable behavior across machines is important.

What are your thoughts?

@nkeim
Copy link
Contributor

nkeim commented Nov 9, 2017

Thanks a ton for shedding some light on this. I agree that this is far from ideal, especially from a scientific reproducibility standpoint.

As a counterargument: I suspect that the vast majority of our users have some kind of comprehensive distribution (e.g. Anaconda) and have skimage, whether they know it or not. Their scripts are all written for non-normalized pixels, and in some cases they care about bit depth and quantization.

What about a one-time warning when pims falls back on matplotlib? We could include a function that suppresses it altogether.

@charlesreid1
Copy link
Contributor Author

charlesreid1 commented Nov 10, 2017

I'm happy to help contribute to such a useful library!

I've submitted a pull request ( #281 ) with a proposed solution: use the warnings module to warn the user when they are using the matplotlib imread() instead of the scikit-image imread().

Here's how the warning will look from an iPython notebook:

img

While this is potentially annoying for users, as it will be printed anytime pims is imported without scikit-image being installed (see below), the user is also helpfully instructed as to how to turn off the warning, and will only be warned once.

img2

If the user has scikit-image installed, no warning will be printed.

Please let me know if this sounds acceptable to you by commenting on this issue or on the PR!

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