-
-
Notifications
You must be signed in to change notification settings - Fork 404
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
feat(bokeh): Server side HoverTool for rasterized/datashaded plots with selector #6422
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6422 +/- ##
==========================================
+ Coverage 88.76% 88.79% +0.02%
==========================================
Files 323 323
Lines 68679 68885 +206
==========================================
+ Hits 60963 61166 +203
- Misses 7716 7719 +3 ☔ View full report in Codecov by Sentry. |
# Conflicts: # holoviews/element/raster.py
# TODO: Investigate why this does not work | ||
# element = element.clone(data=new_data, kdims=element.vdims.copy(), vdims=element.vdims.copy()) | ||
element = element.clone() | ||
element.data = new_data |
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.
Don't understand this.
holoviews/element/raster.py
Outdated
(isinstance(data, dict) and (*map(dimension_name, vdims), alpha.name) in data)): | ||
(isinstance(data, dict) and (*map(dimension_name, vdims), alpha.name) in data) or | ||
str(alpha) in getattr(data, "data_vars", []) | ||
): |
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.
Not that it wasn't already but that's getting quite unwieldy.
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.
Agree.
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.
In that case please refactor 🙂
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'm not sure what you mean. Do you want it to be put into a method?
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.
Refactored in 8f4378e
Which is what I think you mean
Let's remove the RGBA value by default when the index layer is enabled. |
Done in 48f91a5 |
Previously, we sent all hover data to the front end, which could lead to significant overhead on larger plots. With this PR, the data is pushed server-side to the hover tool with a custom data model.
script