spectral extraction: live-preview and export of background spectrum

[kecnry]

Description

This pull request adds live-visualization and export (UI and API) support for the 1D spectrum representing the background region within the spectral extraction plugin.

NOTE: this requires astropy/specreduce#143 (until then, the new test coverage will fail)

Rendered updated plugin docs

Screen.Recording.2022-09-28.at.12.51.06.PM.mov

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.

New Features > Specviz2d:

Spectral extraction plugin now supports visualizing and exporting the 1D spectrum associated with the background region.

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?
• After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?

[codecov]

Codecov Report

Base: 87.63% // Head: 87.64% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (9652679) compared to base (8667847).
Patch coverage: 92.30% of modified lines in pull request are covered.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1682   +/-   ##
=======================================
  Coverage   87.63%   87.64%           
=======================================
  Files          95       95           
  Lines       10131    10154   +23     
=======================================
+ Hits         8878     8899   +21     
- Misses       1253     1255    +2     

Impacted Files	Coverage Δ
...plugins/spectral_extraction/spectral_extraction.py	90.66% <92.30%> (+0.03%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[camipacifici]

How about a thicker solid line for the extracted spectrum and a thin solid line for the background spectrum?

[kecnry]

@camipacifici @PatrickOgle - I actually like the thinner line idea, what do you think? (Here are videos for the dotted and thin styling and how they both react to zooming)

Screen.Recording.2022-10-10.at.2.48.12.PM.mov

Screen.Recording.2022-10-10.at.2.51.36.PM.mov

[camipacifici]

I like the thin line too!

[ojustino]

The code changes seem reasonable to me. A few observations:

1. I am fine with the solid line, too. However, depending on the placement of the initial background region(s), it may not be noticeable if you aren't already looking for it. How do you feel about giving it a different color?

2. When I choose the "Manual" option, I don't see a preview line and the 2D viewer takes 2-3 seconds to adjust when I change the center pixel. (The lag is also present in main.) Exporting a spectrum having chosen "Manual" brings up the "background regions overlapped" error from specreduce. "TwoSided" and "OneSided" are snappy and don't raise any errors for me.

3. Sometimes when I export the background 1D spectrum and then scroll up to change its appearance in Plot Options, the preview line persists in the 1D viewer:

The preview disappears if I scroll back into the Trace or Extraction sections of the Spectral Extraction plugin, so it looks like the trigger is leaving the Spectral Extraction plugin without registering a mouse move (e.g., pressing "Extract" and immediately scrolling up). I'm not sure how many would run into this but thought I'd mention it.

[kecnry]

How do you feel about giving it a different color?

I would like to avoid this, if possible. We use the same color for all "temporary" previews from plugins at the moment.

When I choose the "Manual" option, I don't see a preview line and the 2D viewer takes 2-3 seconds to adjust when I change the center pixel.

I've come across this a few times as well, but is unrelated to this PR (feel free to open an issue if one doesn't already exist).

Sometimes when I export the background 1D spectrum and then scroll up to change its appearance in Plot Options, the preview line persists in the 1D viewer

I suspect the mousemove isn't triggering a change in the "section", but so long as the plugin is opened, I would expect the previews to stay visible, in general. If you moused over a different "section" of the plugin and that wasn't registered, then that may be a bug, although again unrelated to this PR.

[ojustino]

When I choose the "Manual" option, I don't see a preview line and the 2D viewer takes 2-3 seconds to adjust when I change the center pixel.

I've come across this a few times as well, but is unrelated to this PR (feel free to open an issue if one doesn't already exist).

I can see how the lag is unrelated, but do you have thoughts on the "Manual" type's lack of a live preview line and the error upon exporting a spectrum?

[kecnry]

but do you have thoughts on the "Manual" type's lack of a live preview line and the error upon exporting a spectrum?

Addressed in #1737 (and independently upstream in astropy/specreduce#146). Good catch!

[ojustino]

Great, I merged this branch with #1737 and the result fixes both the lag and export error issues with using a "Manual" trace for the background.

Would you consider adjusting the starting pixel for the trace when you select the "Manual" background type? With the image in the example notebook, it starts on the cross-dispersion pixel with the highest flux. This scenario exacerbates the color issue I mentioned earlier since the background's live preview starts off with more flux than the extraction's live preview.

I'll create a separate issue for the persisting preview line.

[kecnry]

Would you consider adjusting the starting pixel for the trace when you select the "Manual" background type? With the image in the example notebook, it starts on the cross-dispersion pixel with the highest flux.

Yes, this currently starts at the same default position that is determined for the initial trace, which probably isn't ideal. I put a proposed improvement to that logic in #1738 (seemed like it should have its own change log entry since it isn't directly related to this PR, even though they're both dealing with the background subtraction step).

[ojustino]

Now that the tests are passing and the spinoff issues are underway, I say this branch is ready for a merge.

[pllim]

Just a very minor nitpick. Code looks like code but should second approval come from a PO?

[pllim]

Ooops change log has conflict now.