Linelist improvements

[duytnguyendtn]

Description

This pull request introduces a series of feature and UI/UX improvements to the Line List plugin. Of note:

1. A new spectral range filter has been introduces to allow filtering of the line list to the observable width of the spectrum viewer. (Responds to changes made to the redshift slider)
2. The Colorpicker has been made collapsible and moved to the title of the line list to better mark the association of the color
3. General padding improvements.

Ran out of time to implement line labels; will need to be pushed to another effort

This entire PR should really be coauthored by @kecnry, who was a big help in getting this PR to this state. Thanks, Kyle!

@PatrickOgle should review this PR, as the designated PO of interest

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 change log needed? If yes, is it added to CHANGES.rst?
• Is a milestone set?
• After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?

[codecov]

Codecov Report

Merging #1327 (e5a741e) into main (3516b14) will increase coverage by 0.78%.
The diff coverage is 87.50%.

@@            Coverage Diff             @@
##             main    #1327      +/-   ##
==========================================
+ Coverage   83.83%   84.61%   +0.78%     
==========================================
  Files          91       91              
  Lines        7751     7846      +95     
==========================================
+ Hits         6498     6639     +141     
+ Misses       1253     1207      -46     

Impacted Files	Coverage Δ
...z/configs/default/plugins/line_lists/line_lists.py	69.35% <87.50%> (+0.47%)	⬆️
jdaviz/configs/cubeviz/plugins/viewers.py	85.41% <0.00%> (-1.69%)	⬇️
jdaviz/configs/cubeviz/plugins/parsers.py	56.29% <0.00%> (-1.57%)	⬇️
...default/plugins/metadata_viewer/metadata_viewer.py	93.40% <0.00%> (-0.83%)	⬇️
jdaviz/configs/mosviz/plugins/parsers.py	90.42% <0.00%> (-0.57%)	⬇️
jdaviz/configs/imviz/plugins/parsers.py	100.00% <0.00%> (ø)
jdaviz/core/astrowidgets_api.py	98.74% <0.00%> (+0.03%)	⬆️
...imviz/plugins/aper_phot_simple/aper_phot_simple.py	91.55% <0.00%> (+0.08%)	⬆️
jdaviz/configs/specviz/plugins/parsers.py	89.02% <0.00%> (+0.41%)	⬆️
jdaviz/utils.py	92.10% <0.00%> (+1.78%)	⬆️
... and 2 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3516b14...e5a741e. Read the comment docs.

[kecnry]

This really does help cleanup the clutter and make the plugin more usable with less wasted space - nice work!

I noticed a few things when doing a quick test... I'll try to do a full review tomorrow:

1. The filtering to lines to the axes limits seems to be overagressive (removing lines still within the limits). I can post a screen recording if you can't reproduce. They do seem to react to changes in the redshift though, so that is awesome!
2. Would it be possible for the plot/erase all buttons to only act on the filtered list of lines rather than the full list? I think that would be a very handy use-case, but if not, maybe the buttons should go on top of the filters to make that more clear (and maybe the tooltip needs clarifying either way).
3. Can the plot/erase all buttons at the bottom of the plugin (which apply to all lists) use the same styling and ordering (there plot is on the right, whereas the new buttons plot is on the left).
4. Do we need to update any of the style guides for this new button style?
5. I'm personally still not a fan of using the same icon for the limits-filter as we do for subsets elsewhere.... but we can leave it as-is for now if and revise if we notice user-confusion.

[duytnguyendtn]

removing lines still within the limits

Hmm yes, if you could send a screenshot that would be awesome! Also if you could call:

plugin = specviz.app.get_tray_item_from_name('g-line-list')
plugin.spectrum_viewer_min, plugin.spectrum_viewer_max

that would be super useful!

plot/erase all buttons to only act on the filtered list

That was the original plan, but I couldn't see a way to actually pull the list of filtered lines from the JS side without relooping through the list again. There wasn't some sort of line.visible attribute, since JS is just creating the UI entry dynamically when needed. This combined with running out of time ended up with me pushing it out.

plot/erase all buttons at the bottom

Good point!

style guides

I'm not sure about this one. Has this created a new paradigm? I don't even know what that would be!

same icon

Working on a new one with Jenn! Here's a sneak peak:

[kecnry]

Hmm yes, if you could send a screenshot that would be awesome!

It seems almost like it is computing the visibility one interaction behind, but I'm not 100% sure. But here you can see two lines within the plot, but only one shown in the sidebar.

plot/erase all buttons to only act on the filtered list

Can the filtering be reproduced on the python side (when clicking the button, if filter_range is True, only act on those that are within the spectrum_viewer_min/max?

Love the direction the new icon is going! I think that'll help make it so much clearer!

[duytnguyendtn]

@kecnry out of curiosity, when the tool is in that state, does toggling the spectral filter correct the results?

[kecnry]

No, it doesn't, but that gave me an idea... and for some reason swapping:

self._viewer.scales['x'].observe(self._on_spectrum_viewer_limits_changed,
                                 names=['min', 'max'])

with

self._viewer.state.add_callback("x_min", lambda x_min: self._on_spectrum_viewer_limits_changed())
self._viewer.state.add_callback("x_max", lambda x_max: self._on_spectrum_viewer_limits_changed())

seems to fix it. The "downside" is that the filter only calculates after you stop dragging, but it seems that it avoids ending in the wrong state. I'm guessing the callbacks on the scales were not processed in a reliable order and so could get confused if you drag back and forth near the edge of the viewer? 🤷

[duytnguyendtn]

Updated the new icon! Thanks @Jenneh!

[PatrickOgle]

UI/UX improvements, including: 1) line padding, 2) hidden color picker, and 3) filter line list to plotted range look great. We might want to consider adding a line below the search that gives the filtered wavelength range and allows that to be edited manually. This could be a separate PR, if desired by more people.

[kecnry]

One small suggestion to avoid headaches in the future, otherwise the code looks super clean, nice work!

[kecnry]

Everything looks good (including in dark mode)! Let's get this in (squashed) once CI passes.

[duytnguyendtn]

Some last padding improvements from @Jenneh to close this PR out!

[Jenneh]

Fantastic visual improvement! :)

[duytnguyendtn]

Thank you everyone for the feedback and reviews! Special thanks to @kecnry and @Jenneh for their patience and guidance through my first plugin-related PR!