Assign Row to NIRISS-parsed 1D Spectra

[duytnguyendtn]

Description

This PR adds a 1 line fix to the NIRISS parser to assign the mosviz_row meta to the 1D spectra.

Because this was such a quick fix, I also took the liberty of cleaning up the niriss and nirspec tests. This was so that I could add a test line to check for this bug in both our NIRISS and NIRspec tests

Fixes #1805

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.15% // Head: 88.15% // Increases project coverage by +0.00% 🎉

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

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1836   +/-   ##
=======================================
  Coverage   88.15%   88.15%           
=======================================
  Files          95       95           
  Lines       10279    10280    +1     
=======================================
+ Hits         9061     9062    +1     
  Misses       1218     1218           

Impacted Files	Coverage Δ
jdaviz/configs/mosviz/plugins/parsers.py	91.13% <100.00%> (+0.02%)	⬆️

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.

[pllim]

👍 for test clean-up. Thanks!

[pllim]

Why is the int casting necessary?

[duytnguyendtn]

So the reasoning here was that I wanted to make sure to avoid situation of None == None . We know the source ids in this dataset are numbers, so I wanted to make sure we were getting back actual values rather than the same, nonsensical thing

[pllim]

Code LGTM. Thanks!

[kecnry]

Awesome, thanks!

[lumberbot-app]

Oops, something went wrong applying the patch ... Please have a look at my logs.

[pllim]

Hmm, auto backport failed. Do we really need to backport this? If so, @duytnguyendtn will need to manually do it.

[pllim]

(I alerted the backport bot dev on how it is no longer applying follow-up label and proper manual backport message on failed backport.)

[duytnguyendtn]

I'm not particularly tied to this being backport @pllim. This bug has been in mosviz for quite some time (over a year) so I can't imagine anyone's desperate to get this fixed ASAP. It doesn't impact mosviz's core functionality.

What should we do with the existing milestones? How do we switch this back to 3.2 AFTER the merge? is it just as simple as changing the milestone and removing the backport label?

[pllim]

It is! I did that for you. 😸

[duytnguyendtn]

I guess we also need to move the changelog entry. Should I make a PR for that?

[pllim]

@duytnguyendtn , do you want to open a follow-up PR to move the change log to the 3.2 section instead?

[duytnguyendtn]

Will do!

[duytnguyendtn]

Thanks Pey-Lian! I'll have a followup PR ready soon!

[pllim]

Just testing the bot, please ignore.

@meeseeksdev backport to v3.1.x

[lumberbot-app]

Oops, something went wrong applying the patch ... Please have a look at my logs.

[blink1073]

@meeseeksdev backport to v3.1.x

[lumberbot-app]

Awww, sorry blink1073 you do not seem to be allowed to do that, please ask a repository maintainer.

[pllim]

Just testing the bot, please ignore. (Round 2)

@meeseeksdev backport to v3.1.x

[lumberbot-app]

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

git checkout v3.1.x
git pull

2. Cherry pick the first parent branch of the this PR on top of the older branch:

git cherry-pick -x -m1 b28ad12ed1a823a8579a03f4eb73ac19b250507a

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

git commit -am 'Backport PR #1836: Assign Row to NIRISS-parsed 1D Spectra'

4. Push to a named branch:

git push YOURFORK v3.1.x:auto-backport-of-pr-1836-on-v3.1.x

5. Create a PR against branch v3.1.x, I would have named this PR:

"Backport PR #1836 on branch v3.1.x (Assign Row to NIRISS-parsed 1D Spectra)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.