Fixes to ensure streams are correctly mapped to plots #6415
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6415 +/- ##
==========================================
+ Coverage 88.48% 88.50% +0.01%
==========================================
Files 323 323
Lines 68469 68599 +130
==========================================
+ Hits 60588 60711 +123
- Misses 7881 7888 +7 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Very happy to see #5595 being fixed! Overall LGTM (made some minor optional suggestions).
I was not able to see whether the OP example in #5434 is fixed as there seems to be some issues with the box edit tool, I can't create any selection.
@philippjfr make sure the PR title is updated based on your last changes since it seems the recursion is still done.
Technically spit_dmap_overlay is public as documented on the website (https://holoviews.org/reference_manual/holoviews.plotting.html#holoviews.plotting.util.split_dmap_overlay) so that's a breaking change. No one's probably using so I don't think it's an issue, the release notes might just include a comment on that.
| @@ -202,6 +202,13 @@ def test_dynamic_compute_overlayable_zorders_mixed_dynamic_and_non_dynamic_ndove | |||
| self.assertIn(curve, sources[2]) | |||
| self.assertNotIn(ndoverlay, sources[2]) | |||
|
|
|||
| def test_dynamic_compute_overlayable_zorders_ndoverlays_as_input(self): | |||
There was a problem hiding this comment.
Does this test cover the two bugs fixed by the PR?
There was a problem hiding this comment.
Had real trouble finding a smaller reproducible example for the first issue.
The UX of drawing boxes has changed in Bokeh (in quite a non-obvious way). You have to hold down shift and then draw. |
Co-authored-by: Maxime Liquet <35924738+maximlt@users.noreply.github.com>
philippjfr
changed the title
|
I'm going to merge this and follow up with tests in a bit as I have to get the RC out asap. |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This fix addresses a tricky situation where nested overlays were not resolved correctly because it would simply keep recursing until it got to the last layer. This meant the resulting subplot(s) would not update. We now still do the recursing (though if I spent a lot more time on this we maybe wouldn't have to do so), but also record all the streams along the way.
Also fixes a separate bug where during zorder assignment we did not take into account DynamicMaps that had an (Nd)Overlay as input, which would confuse the zorder assignment and therefore meant that streams would be assigned to the wrong plot (or no plot at all).
Fixes #5595
Fixes #5434