-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Imshow #1855
Imshow #1855
Conversation
@@ -0,0 +1,122 @@ | |||
import plotly.graph_objs as go | |||
import numpy as np # is it fine to depend on np here? |
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.
it is fine, but we should have a little friendly error message if it's not installed, similar to the pandas one for px, no?
in fact, px depends on numpy but doesn't have a friendly error message... maybe we can do both in a separate PR.
How about: let's open a separate issue to discuss the numpy
dependency and move forward with this as is :)
if dt in _integer_types: | ||
return _integer_ranges[dt][1] | ||
else: | ||
return img[np.isfinite(img)].max() |
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.
Per our conversation, I think this should return "the smallest power of 255 which is greater than the max".
My rationale here is that if you have a pipeline that's intended to produce output between 0-X, and you use this to display the output of multiple inputs, it would be nice for them to have the same bounds, instead of having the bounds vary. And it feels like the most likely values of X are 1 and 255, and then possibly thereafter some powers thereof, if the data is 16-bit or 32-bit or whatnot :)
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.
one thing which bothers me though is the following case: suppose an image is in the [0-1] range but some numerical computation (filter etc.) changes the max to 1 + some small value. Then the zmax will be 255 for a max of say 1.05, and it will look really bad.
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.
I can add a multiplicative tolerance factor like 10%
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.
I'd be OK with this
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.
how about having a keyword argument colorrange
(or a better name consistent maybe with other parts of the library) which could be data
(take min/max) or dtype
(inference based on what we've discussed)? With this I think most people would be happy... Or does this complicate too much the API ?
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.
My feeling is that that complicates too much
OK, so here is a summary of the zmax behaviour
In all cases, the user can override the zmax value. |
Awesome! It's nice to have someone who knows this world on the team :) 💃 |
This is still WIP, I need to track some corner cases, but this is to give an idea of what the
imshow
function could look like.