-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 support for zooming sub-plots #13345
Conversation
Really cool! Are
|
Codecov Report
@@ Coverage Diff @@
## branch-3.3 #13345 +/- ##
===========================================
Coverage 92.45% 92.45%
===========================================
Files 317 317
Lines 20268 20275 +7
===========================================
+ Hits 18738 18745 +7
Misses 1530 1530 |
Looking great! |
79339bb
to
6802a8a
Compare
This is tentatively ready for review. With this PR |
This should be discussed/performed in PR #13346. I'm generally not opposed to renaming. The current naming convention is taken from scales where we have |
Hi @mattpap , could you provide code for a working example? I'm getting |
@droumis, see the example (examples/interaction/tools/subcoordinates_zoom.py) added in this PR.
This is a mismatch between bokeh and bokehjs. |
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.
Looks good to me. It would be good to get @droumis' approval that this provides all the required functionality.
apart from my comment above, all looks good to go |
|
||
target_scale = Required(Instance(Scale), help=""" | ||
The target scale. | ||
""") |
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.
These help strings don't add anything, really. What would be helpful for both of these is a brief concrete example to anchor the concepts, e.g. if we have a plot and a subplot, which one of those is the source and which one is the target?
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 will continue documenting sub-coordinates' related stuff in PR #13346.
Co-authored-by: Demetris Roumis <roumis.d@gmail.com>
d13d567
to
5e7a667
Compare
This PR does two things:
Example. Notice that zoom in/out works on x frame range, however not y frame range, but y sub-coordinate ranges:
Screencast_00003.mp4
Code
TODO:
fixes #13339