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-2066: Add srctype to Combine1D headers #6079

Merged
merged 3 commits into from
May 27, 2021

Conversation

tapastro
Copy link
Contributor

@tapastro tapastro commented May 26, 2021

Closes #6000 🎉

Resolves JP-2066

Description

This PR adds source_type information to combine1d extensions of c1d.fits files, pulled from the x1d inputs to the Combine1D step.

Checklist

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

@tapastro tapastro added this to the Build 7.9 milestone May 26, 2021
@codecov
Copy link

codecov bot commented May 26, 2021

Codecov Report

Merging #6079 (ff171c6) into master (20d5bbb) will increase coverage by 0.03%.
The diff coverage is 100.00%.

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

@@            Coverage Diff             @@
##           master    #6079      +/-   ##
==========================================
+ Coverage   76.99%   77.03%   +0.03%     
==========================================
  Files         402      402              
  Lines       34329    34406      +77     
==========================================
+ Hits        26431    26503      +72     
- Misses       7898     7903       +5     
Flag Coverage Δ *Carryforward flag
nightly 76.99% <100.00%> (ø) Carriedforward from 20d5bbb
unit 55.17% <100.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
jwst/combine_1d/combine1d.py 90.59% <100.00%> (+0.81%) ⬆️

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 20d5bbb...c1d7905. Read the comment docs.

@tapastro
Copy link
Contributor Author

I'll be honest that I never really looked into why there are a few different CI test runs - any explanation why the SDP deps version failed here? I poked around a bit, and it seems like things might be out of date in that run, but I don't fully grok what's going on here.

@hbushouse hbushouse modified the milestones: Build 7.9, Build 7.8 May 26, 2021
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.

LGTM

@hbushouse
Copy link
Collaborator

I'll be honest that I never really looked into why there are a few different CI test runs - any explanation why the SDP deps version failed here? I poked around a bit, and it seems like things might be out of date in that run, but I don't fully grok what's going on here.

Unfortunately the previous CI results seem to have disappeared, as a result of the latest commit using [skip ci]

@tapastro
Copy link
Contributor Author

I'll be honest that I never really looked into why there are a few different CI test runs - any explanation why the SDP deps version failed here? I poked around a bit, and it seems like things might be out of date in that run, but I don't fully grok what's going on here.

Unfortunately the previous CI results seem to have disappeared, as a result of the latest commit using [skip ci]

Under the checks tab, you should be able to select the checks of the previous commit with a dropdown - it failed one of them.

@hbushouse
Copy link
Collaborator

Hmmm, same set of tests failed in my PR for the CI run using SDP dependencies. No clue.

@hbushouse
Copy link
Collaborator

CI failure for "SDP dependencies" test is due to use of the latest CRDS context that has a bad NIRCam specwcs rmap in it. Redcat folks are fixing. So you can ignore.

@tapastro tapastro merged commit b77df8a into spacetelescope:master May 27, 2021
@tapastro tapastro deleted the jp-2066-srctype-c1d-headers branch May 27, 2021 12:46
AntoineDarveau pushed a commit to talensgj/jwst that referenced this pull request Oct 26, 2021
* start adding source_type

* push source_type to c1d

* [skip ci] changelog
loicalbert pushed a commit to talensgj/jwst that referenced this pull request Nov 5, 2021
* start adding source_type

* push source_type to c1d

* [skip ci] changelog
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.

Propagate SRCTYPE to c1d headers
2 participants