Interactive spline

[haticekaratay]

Description

This draft pull request is to test Tom's implementation for enabling custom stretches. At the moment, the setters for the knots haven't been finalized yet. However, we can currently set the knots' (x, y) values dynamically and observe the updates of both x and y on the image viewer and the histogram.

viewer.state.layers[0].stretch = 'spline'
viewer.state.layers[0].stretch_parameters['knots'] = ([0,0.2,0.4,1.0],[0, 0.3, 0.5, 1.0])
viewer.state.layers[0].stretch_object
viewer.state.layers[0].stretch_object.knots

Fixes #

Change log entry

• Is a change log needed? If yes, is it added to CHANGES.rst? If you want to avoid merge conflicts,
  list the proposed change log here for review and add to CHANGES.rst before merge. If no, maintainer
  should add a no-changelog-entry-needed label.

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

• Are two approvals required? Branch protection rule does not check for the second approval. If a second approval is not necessary, please apply the trivial label.
• Do the proposed changes actually accomplish desired goals? Also manually run the affected example notebooks, if necessary.
• Do the proposed changes follow the STScI Style Guides?
• Are tests added/updated as required? If so, do they follow the STScI Style Guides?
• Are docs added/updated as required? If so, do they follow the STScI Style Guides?
• Did the CI pass? If not, are the failures related?
• Is a milestone set? Set this to bugfix milestone if this is a bug fix and needs to be released ASAP; otherwise, set this to the next major release milestone.
• After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?

[rosteen]

I tried testing this, and hit the following error immediately upon opening the Plot Options plugin:

File ~/projects/jdaviz/jdaviz/configs/default/plugins/plot_options/plot_options.py:829, in PlotOptions._update_stretch_histogram(self, msg)
    827 self._histogram_nbins_changed()
    828 # update the curve/colorbar
--> 829 self._update_stretch_curve(msg)

File ~/projects/jdaviz/jdaviz/core/template_mixin.py:268, in skip_if_no_updates_since_last_active.<locals>.decorator.<locals>.wrapper(self, msg)
    266 if meth.__name__ not in self._methods_skip_since_last_active:
    267     self._methods_skip_since_last_active.append(meth.__name__)
--> 268 ret_ = meth(self, msg)
    269 if ret_ is False:
    270     self._methods_skip_since_last_active.remove(meth.__name__)

File ~/projects/jdaviz/jdaviz/configs/default/plugins/plot_options/plot_options.py:867, in PlotOptions._update_stretch_curve(self, msg)
    864 contrast_bias = ContrastBiasStretch(self.image_contrast_value, self.image_bias_value)
    866 # get the stretch object
--> 867 stretch = layer.state.stretch_object
    868 layer_cmap = layer.state.cmap
    870 # show the colorbar

AttributeError: 'BqplotImageLayerState' object has no attribute 'stretch_object'

Is layer.state.stretch_object supposed to get set somewhere by us, or do I need to upgrade a package version? My glue- and bqplot-related packages seem to be up to date.

Edit: Ah, looks like I might need an unreleased PR of Tom's?

[haticekaratay]

I tried testing this, and hit the following error immediately upon opening the Plot Options plugin:

File ~/projects/jdaviz/jdaviz/configs/default/plugins/plot_options/plot_options.py:829, in PlotOptions._update_stretch_histogram(self, msg)
    827 self._histogram_nbins_changed()
    828 # update the curve/colorbar
--> 829 self._update_stretch_curve(msg)

File ~/projects/jdaviz/jdaviz/core/template_mixin.py:268, in skip_if_no_updates_since_last_active.<locals>.decorator.<locals>.wrapper(self, msg)
    266 if meth.__name__ not in self._methods_skip_since_last_active:
    267     self._methods_skip_since_last_active.append(meth.__name__)
--> 268 ret_ = meth(self, msg)
    269 if ret_ is False:
    270     self._methods_skip_since_last_active.remove(meth.__name__)

File ~/projects/jdaviz/jdaviz/configs/default/plugins/plot_options/plot_options.py:867, in PlotOptions._update_stretch_curve(self, msg)
    864 contrast_bias = ContrastBiasStretch(self.image_contrast_value, self.image_bias_value)
    866 # get the stretch object
--> 867 stretch = layer.state.stretch_object
    868 layer_cmap = layer.state.cmap
    870 # show the colorbar

AttributeError: 'BqplotImageLayerState' object has no attribute 'stretch_object'

Is layer.state.stretch_object supposed to get set somewhere by us, or do I need to upgrade a package version? My glue- and bqplot-related packages seem to be up to date.

Edit: Ah, looks like I might need an unreleased PR of Tom's?

Thanks @rosteen for looking into this. Yes, to test this PR, the following need to be added to the env. Pip install git+https://github.com/glue-viz/glue-jupyter.git@refs/pull/409/merge Pip install git+https://github.com/glue-viz/glue.git@refs/pull/2453/merge

[rosteen]

Alright, once I got the required glue and glue-jupyter things installed, this works pretty well! The performance isn't totally smooth, but I think that's to be expected considering we have to recalculate the interpolation every time a knot is moved slightly. The only case I found that I think needs to be fixed is that if you select one of the preset stretch percentile options after messing around with the knots a bit, you can get into a state where the interpolation doesn't pass through the knots anymore:

I think the solution here is probably as simple as triggering the interpolation update (I guess that's in update_knots) on updating that menu selection. Fingers crossed.

[codecov]

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (bd312bd) 91.51% compared to head (6cd8fb7) 91.50%.
Report is 15 commits behind head on main.

❗ Current head 6cd8fb7 differs from pull request most recent head 9d74541. Consider uploading reports for the commit 9d74541 to get more accurate results

Files	Patch %	Lines
jdaviz/core/tools.py	78.04%	9 Missing ⚠️
...nfigs/default/plugins/plot_options/plot_options.py	95.00%	1 Missing ⚠️
jdaviz/core/template_mixin.py	90.90%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2545      +/-   ##
==========================================
- Coverage   91.51%   91.50%   -0.01%     
==========================================
  Files         161      161              
  Lines       19596    19809     +213     
==========================================
+ Hits        17934    18127     +193     
- Misses       1662     1682      +20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rosteen]

I still kind of wish that the spline knot dragging was a separate tool from the vmin/vmax dragging, but I'm willing to approve this and potentially revisit later if other people end up finding the unified tool annoying to use. The main issue for me is that trying to adjust the location of the first free knot often ends up snapping to adjusting the vmin instead - in one case I actually clicked directly on the knot and it still adjusted the vmin instead:

Screen.Recording.2023-12-07.at.11.17.41.AM.mov

I think this may be in part due to the throttling, such that the user moves their mouse too fast and the location of the knot doesn't get updated enough to keep up with the mouse and stay closer than the bound when dragging. We could probably decrease the throttle a little bit (I tested at 0.02 instead of 0.05 and it seemed maybe a little better). I'm also wondering if a discrepancy in the calculations for distance to bound vs distance to knot might be exacerbating - it looks like the distance to the knot is normalized, but the distance to the bound is not. Is that accurate? Actually, I'll just comment on those lines...stand by...

[rosteen]

I just tested this, and it looks like we do need to normalize the x distances as well - if you test in the Cubeviz example notebook, you can see that even clicking as close as possible to the vmin bound gives closest_bound_distance values of around 1 to 10, which means the <0.1 condition isn't being met (which makes it impossible to move).

[rosteen]

Updating the vmin and vmax in Cubeviz works now, thanks!

[bmorris3]

PCHIP interp looks better, thank you!