-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add stretch options for visualizing images #1803
Conversation
Setting the new min value before the new max value can lead to a TraitError in the case where the new min is greater than the old max. To prevent that, this checks for that case and reverses the order that attrs are set in if needed.
@aazuspan Great work! I don't think we need to implement the same functionality for folium as it does not support ipywidgets. The unit test errors are irrelevant to this PR. I just fixed it. |
bestEffort=True, | ||
maxPixels=10_000, | ||
crs="SR-ORG:6627", | ||
scale=1, |
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.
Should the scale
be the native resolution of the image insteaded of being hardcoded as 1
?
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.
Good question! I went with scale=1
and the weird CRS because they're used by the Code Editor for calculating vis params (screenshot of the relevant obfuscated code from playground.js
below). I'm guessing a fixed scale was used to avoid tricky corner cases like images with different projections per band or images with zero bands, which would break if you tried to use img.projection().nominalScale()
and img.select(0).projection().nominalScale()
, respectively. I'm open to other ideas though!
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.
Makes sense! Thanks for looking into this. I did not know that the Code Editor actually use these parameters.
geometry=bounds, | ||
bestEffort=True, | ||
maxPixels=10_000, | ||
crs="SR-ORG:6627", |
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.
Maybe EPSG:3857
is more common? @jdbcode
This is a great additon! I really like it. Thank you for your contribution. |
Thanks for the quick review! |
Demo raster_vis.mp4 |
@aazuspan this is so great! Thanks for implementing it! 98% is my go-to in the code editor - so handy for debugging and quick reality check - love that I can do it in geemap now. Regarding the reducer scale and crs, I think following what the Code Editor does is a fine solution. My instinct would have been to set scale based on the map zoom level, but parity with Code Editor is nice and they've probably worked through any issues with optimization/result. 😀🙏🎉❤️🚀 |
This would close #1617 by:
EELeafletTileLayer
method for calculating min and max vis params using stretches_RasterLayerEditor
to select and apply stretches@jdbcode and @giswqs I hope you don't mind I went ahead and put together a prototype. It seemed like the discussion in #1617 might be easier if we had some code to play around with. For this first pass, I basically copied the Code Editor functionality in terms of how image stats are calculated (after poking around in the network requests to see how exactly it works), but I'm open to suggestions if you think we should deviate.
In terms of implementation, I put the core calculations in
EELeafletTileLayer
to allow calculating params outside of the UI, as suggested by @jdbcode, and to permit layer-level caching of vis stats. Currently, that methodcalculate_vis_minmax
just returns a min/max tuple that's used by the UI to set the range, rather than storing avis_params
attribute on that class and updating theurl_format
, as was discussed in #1617. I was hesitant to add that due to the redundancy with_RasterLayerEditor._vis_params
, but I'm happy to refactor if we want to go with that approach.Another thing I wasn't clear on is whether this needs to be re-implemented for
EEFoliumTileLayer
? If so, maybe we need a base classEETileLayer
that would implement the vis params and URL formatting, andEELeafletTileLayer
andEEFoliumTileLayer
could inherit from that as well as their respective superclasses?The one substantial UX difference I made from the Code Editor is the "Refresh" button. In the Code Editor, clicking the same stretch option after adjusting bands or changing the map bounds will re-calculate the stretch, but
ipywidgets
doesn't register an event if you select the same option in a dropdown, so getting identical functionality wasn't possible. The refresh button was a workaround to allow manually triggering recalculation without having to select a different option first. Curious to hear if anyone has other ideas!