Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JP-2013 NIRSpec leakcal and nodded exposures #7426

Merged
merged 8 commits into from
Jan 18, 2023

Conversation

jemorrison
Copy link
Collaborator

@jemorrison jemorrison commented Jan 6, 2023

Resolves JP-2013

Closes #5911

This PR follows changes made in how level-2b associations are created for NIRSpec exposures with both leakcal and nodded exposures (#7405). The calspec2 pipeline has been changed to subtract leakcal images from science and background data before the background step is run on the science data.

Checklist for maintainers

  • added entry in CHANGES.rst within the relevant release section
  • updated or added relevant tests
  • updated relevant documentation
  • added relevant milestone
  • added relevant label(s)
  • ran regression tests, post a link to the Jenkins job below.
    [How to run regression tests on a PR]: results

https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/534/

@jemorrison
Copy link
Collaborator Author

Running regressions tests now

@codecov
Copy link

codecov bot commented Jan 9, 2023

Codecov Report

Base: 78.61% // Head: 78.51% // Decreases project coverage by -0.10% ⚠️

Coverage data is based on head (355b92c) compared to base (d89abc0).
Patch coverage: 59.25% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7426      +/-   ##
==========================================
- Coverage   78.61%   78.51%   -0.11%     
==========================================
  Files         455      455              
  Lines       39143    39211      +68     
==========================================
+ Hits        30773    30787      +14     
- Misses       8370     8424      +54     
Flag Coverage Δ
nightly 78.59% <59.25%> (-0.02%) ⬇️
unit 51.41% <40.74%> (+0.06%) ⬆️
Impacted Files Coverage Δ
jwst/imprint/imprint_step.py 70.96% <52.63%> (-29.04%) ⬇️
jwst/pipeline/calwebb_spec2.py 94.20% <75.00%> (+0.06%) ⬆️
jwst/wfss_contam/observations.py 47.36% <0.00%> (-10.41%) ⬇️
jwst/regtest/regtestdata.py 86.23% <0.00%> (-0.46%) ⬇️

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.

@jemorrison
Copy link
Collaborator Author

The regression test show 8 failures. 4 failures are from tests that has an extra keyword S_IMPRINT SKIPPED.
In those 4 causes the imprint step should not be run because the data is not NIRSPEC IFU data. I am not sure why the S_IMPRINT was not set to SKIP before. I down loaded on of the test data: test_miri_lrs_slit_spec. I ran calspec2 on the data and by default the IMPRINT step is set to skip. Now it is written in the header and before it was not. Maybe something else changes to does this now ?
The 4 other tests are related to NIRISS SOSS and I do not think are related to this PR.

docs/jwst/imprint/description.rst Outdated Show resolved Hide resolved
jwst/pipeline/calwebb_spec2.py Outdated Show resolved Hide resolved
jwst/pipeline/calwebb_spec2.py Outdated Show resolved Hide resolved
jwst/pipeline/calwebb_spec2.py Show resolved Hide resolved
@jemorrison
Copy link
Collaborator Author

I re-ran the regression tests and now there are only 4 "failures" - these are just 1 difference in header that now has the IMPRINT step set to skipped. In these cases it should be set to skipped because the data is not NIRSpec IFU data

Copy link
Collaborator

@stscieisenhamer stscieisenhamer left a comment

Choose a reason for hiding this comment

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

LGTM.

@hbushouse hbushouse merged commit 093ec3e into spacetelescope:master Jan 18, 2023
@jemorrison jemorrison deleted the JP-2013_leakcal branch July 17, 2023 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate ASN rules for NIRSpec leakcal and nodded exposures
3 participants