Skip to content

Add wcsinfo to multislitmodel schema#712

Closed
emolter wants to merge 4 commits into
spacetelescope:mainfrom
emolter:multislit-wcsinfo
Closed

Add wcsinfo to multislitmodel schema#712
emolter wants to merge 4 commits into
spacetelescope:mainfrom
emolter:multislit-wcsinfo

Conversation

@emolter
Copy link
Copy Markdown
Contributor

@emolter emolter commented Apr 8, 2026

Required to merge #710 and see discussion on that PR.

This PR adds wcsinfo to the MultiSlitModel schema. It turns out that we request wcsinfo from MultiSlitModel in the pipeline and it seems it should therefore be part of the expected metadata (and as such get saved with the outputs).

Tasks

  • update or add relevant tests
  • update relevant docstrings and / or docs/ page
  • Does this PR change any API used downstream? (if not, label with no-changelog-entry-needed)
    • write news fragment(s) in changes/: echo "changed something" > changes/<PR#>.<changetype>.rst (see below for change types)
    • run jwst regression tests with this branch installed ("git+https://github.com/<fork>/stdatamodels@<branch>")
news fragment change types...
  • changes/<PR#>.feature.rst: new feature
  • changes/<PR#>.bugfix.rst: fixes an issue
  • changes/<PR#>.doc.rst: documentation change
  • changes/<PR#>.removal.rst: deprecation or removal of public API
  • changes/<PR#>.misc.rst: infrastructure or miscellaneous change

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.33%. Comparing base (75c2006) to head (8a459b1).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #712   +/-   ##
=======================================
  Coverage   90.32%   90.33%           
=======================================
  Files          99       99           
  Lines        4641     4644    +3     
=======================================
+ Hits         4192     4195    +3     
  Misses        449      449           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@emolter
Copy link
Copy Markdown
Contributor Author

emolter commented Apr 8, 2026

https://github.com/spacetelescope/RegressionTests/actions/runs/24156781396

There is a lot of new metadata for this change. I wonder if we are generally ok with that or not.

@emolter emolter mentioned this pull request Apr 8, 2026
5 tasks
@emolter emolter force-pushed the multislit-wcsinfo branch from 8ec098f to ad4bd1e Compare April 17, 2026 14:08
@emolter emolter marked this pull request as ready for review April 17, 2026 14:10
@emolter emolter requested a review from a team as a code owner April 17, 2026 14:10
@melanieclarke
Copy link
Copy Markdown
Contributor

https://github.com/spacetelescope/RegressionTests/actions/runs/24156781396

There is a lot of new metadata for this change. I wonder if we are generally ok with that or not.

The NIRSpec spec2 changes look okay – the only difference is in some empty comment-style headers.

But it looks like this is now writing an imaging-type FITS WCS to WFSS and NIRCam DHS exposures, which isn’t right. That will probably need explicit handling.

@emolter emolter marked this pull request as draft April 17, 2026 14:23
Copy link
Copy Markdown
Contributor

@melanieclarke melanieclarke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this set looks good, except possibly for the SYS keywords. RADESYS, VELOSYS, and SPECSYS are FITS WCS keywords. I don't think they'd do any harm on their own, but they shouldn't be needed, detached from any other WCS keywords.

@emolter
Copy link
Copy Markdown
Contributor Author

emolter commented Apr 17, 2026

ok, thanks. I left them in because they do exist already in the NIRSpec multislitmodel headers, so removing them would (along with the proposed model.update changes) lead to them no longer being there. I assume based on your comment we are ok with that.

@melanieclarke
Copy link
Copy Markdown
Contributor

ok, thanks. I left them in because they do exist already in the NIRSpec multislitmodel headers, so removing them would (along with the proposed model.update changes) lead to them no longer being there. I assume based on your comment we are ok with that.

Oh, good to know, thanks. If they're there currently, let's keep them just in case.

@emolter emolter mentioned this pull request Apr 22, 2026
10 tasks
@emolter
Copy link
Copy Markdown
Contributor Author

emolter commented Apr 22, 2026

spacetelescope/jwst#10464 handles this differently, moving relevant metadata into the slits of the multislitmodel instead of updating the multislitmodel schema.

@emolter emolter closed this Apr 22, 2026
@emolter emolter deleted the multislit-wcsinfo branch April 22, 2026 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants