Bugfix: skip color cycler in Mosviz

[bmorris3]

Description

The color cycler update in #1866 unintentionally changes the color of each spectrum visualized with Mosviz. Though the colors are fun, this isn't useful or intended.

This PR only applies the color cycler for configs other than Mosviz. For Mosviz, only the default (gray) color is used.

I've tested locally that Mosviz produces gray lines in the spectrum viewer, and e.g. Cubeviz still cycles colors.

🐱

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)?

[codecov]

Codecov Report

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

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

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1900   +/-   ##
=======================================
  Coverage   88.48%   88.48%           
=======================================
  Files          95       95           
  Lines       10540    10543    +3     
=======================================
+ Hits         9326     9329    +3     
  Misses       1214     1214           

Impacted Files	Coverage Δ
jdaviz/app.py	94.57% <ø> (ø)
jdaviz/configs/mosviz/plugins/viewers.py	87.67% <100.00%> (+0.11%)	⬆️
jdaviz/utils.py	90.16% <100.00%> (+0.08%)	⬆️

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.

[bmorris3]

@kecnry pointed out that we still want plugins in Mosviz to apply different colors – for example, model fits should still have color. We only want to override the color cycler while selecting different rows. 7c80f2e implements this by manipulating the color cycler from within the row selection methods in the Mosviz table viewer.

[kecnry]

Interesting solution! I'm wondering if just resetting to 0 would be more general (if adding multiple other entries for the current row, the next row click should still start at gray)?

[bmorris3]

Good call – in testing I was able to switch between rows with models in such a way that the selected row's spectrum was shown in color other than gray. I'm about to push a commit that sets the counter = -1 -- calls to the cycler always increment the counter by one, even on the zeroth call.

[kecnry]

Elegant fix! Some day it would be nice for layers to retain their plot options when changed by the user, in which case this logic might need to change a little... but that is definitely out-of-scope here.

(Would be nice to have green CI before merge, but failure should be unrelated and don't expect this PR to cause any failures)

[duytnguyendtn]

Confirmed grey only in mosviz, but color cycles in specviz.

Agreed with Kyle, very elegant! It took me a second to read through the code and find where the solution was. Originally I was asking myself, "wait, where's the fix here?" 😅

[kecnry]

I'd vote for a squash here to get the other attempts out of the history on main... not sure if squash & merge plays nicely with the automatic backporter or if it needs to be squashed in advance 🤷

[rosteen]

I'd vote for a squash here to get the other attempts out of the history on main... not sure if squash & merge plays nicely with the automatic backporter or if it needs to be squashed in advance 🤷

I seem to remember that they don't play well together, need to squash in advance.