improve initial scatter/mouseover performance #137
Conversation
kecnry
force-pushed
the
mouseover-performance
branch
from
4b91d54 to
6702dd7
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #137 +/- ##
==========================================
- Coverage 93.92% 91.20% -2.73%
==========================================
Files 37 41 +4
Lines 1598 2148 +550
==========================================
+ Hits 1501 1959 +458
- Misses 97 189 +92 ☔ View full report in Codecov by Sentry. |
| @@ -149,6 +149,7 @@ def _apply_layer_defaults(self, layer_state): | |||
| if getattr(layer_state.layer, 'meta', {}).get('Plugin', None) == 'Binning': | |||
| # increased size of binned results, by default | |||
| layer_state.size = 5 | |||
| layer_state.points_mode = 'markers' | |||
There was a problem hiding this comment.
this isn't needed in order to get the improvements from glue-jupyter, but doesn't hurt! If we ever have light curves with enough points where the scatter with markers becomes more expensive than the density plot, we can consider removing this to leave it at the default of 'auto'.
| # glue-jupyter 1.18 changed from lyr.scatter to lyr.scatter_mark | ||
| # TODO: once glue-jupyter is pinned to 1.18 or later, update this to: | ||
| # scatter = lyr.scatter_mark | ||
| scatter = getattr(lyr, 'scatter_mark', getattr(lyr, 'scatter', None)) | ||
| scatter = lyr.scatter_mark |
There was a problem hiding this comment.
lcviz currently requires jdaviz 3.10, which in turn requires glue-jupyter 0.20+ (original comment was a typo glue-jupyter made this change in 0.18, not 1.18, with glue-viz/glue-jupyter#363)
kecnry
changed the title
Co-authored-by: Kyle Conroy <[email protected]>
This along with glue-viz/glue-jupyter#467 (included in glue-jupyter 0.22.2) improves the performance at load time, especially when the server is remote.
Note that lcviz does not explicitly pin glue-jupyter (it pulls in glue-jupyter through jdaviz).