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-1822: Update NRS fixed slit association rules for background eligibility #5994

Merged

Conversation

tapastro
Copy link
Contributor

@tapastro tapastro commented Apr 28, 2021

Closes #5547

Resolves JP-1822

Description

This PR addresses the usage of science exposures as backgrounds for other science exposures in an observation with fixed slit nods; this is accomplished by using the SPAT_NUM keyword to match exposures with different spatial nod positions for use as backgrounds in each other's associations.

NOTE: Waiting for an up-to-date pool file for testing before merge.

Checklist

  • Tests
  • Documentation
  • Change log
  • Milestone
  • Label(s)

@tapastro tapastro added this to the Build 7.8 milestone Apr 28, 2021
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.

Obligatory "don't forget the changelog"

@tapastro
Copy link
Contributor Author

Obligatory "don't forget the changelog"

I modified the jw00626 pool file to contain SPAT_NUM, and it runs successfully. However, the two unit tests with coverage of this edit are failing, so I think I may have a bit of maintenance to take care of first.

Copy link
Collaborator

@hbushouse hbushouse left a comment

Choose a reason for hiding this comment

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

Updates to the actual ASN code look good. Of course unit tests need fixing now.

@codecov
Copy link

codecov bot commented May 11, 2021

Codecov Report

Merging #5994 (ef24157) into master (6609329) will decrease coverage by 0.16%.
The diff coverage is 79.16%.

❗ Current head ef24157 differs from pull request most recent head 0134bba. Consider uploading reports for the commit 0134bba to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5994      +/-   ##
==========================================
- Coverage   77.81%   77.65%   -0.17%     
==========================================
  Files         399      405       +6     
  Lines       35354    36726    +1372     
==========================================
+ Hits        27512    28519    +1007     
- Misses       7842     8207     +365     
Flag Coverage Δ *Carryforward flag
nightly 77.82% <64.70%> (ø) Carriedforward from 6609329
unit 56.66% <63.63%> (-0.21%) ⬇️

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
jwst/associations/lib/rules_level2_base.py 84.50% <79.16%> (-0.28%) ⬇️
jwst/pipeline/calwebb_spec2.py 76.74% <0.00%> (-19.32%) ⬇️
jwst/ramp_fitting/utils.py 92.40% <0.00%> (-1.85%) ⬇️
jwst/source_catalog/source_catalog_step.py 89.06% <0.00%> (-1.27%) ⬇️
jwst/step.py 100.00% <0.00%> (ø)
jwst/ramp_fitting/ramp_fit.py 100.00% <0.00%> (ø)
jwst/wfss_contam/__init__.py 100.00% <0.00%> (ø)
jwst/wfss_contam/wfss_contam.py 21.42% <0.00%> (ø)
jwst/wfss_contam/sens1d.py 25.92% <0.00%> (ø)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6609329...0134bba. Read the comment docs.

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.

obligatory "update changelog" but LGTM.

@tapastro tapastro merged commit 6c14181 into spacetelescope:master May 19, 2021
@tapastro tapastro deleted the jp-1822-nrsfss-bkgd-asn-rules branch May 19, 2021 17:39
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.

Update NRS FS background association rules per JWSTDMS-325
3 participants