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

Set_telescope_pointing to handle zero values for pointing mnemonics in engineering database #5540

Merged
merged 3 commits into from Dec 16, 2020

Conversation

stscirij
Copy link
Contributor

Addresses #5453 and JP-1771

@codecov
Copy link

codecov bot commented Dec 16, 2020

Codecov Report

Merging #5540 (a49d192) into master (202b32e) will increase coverage by 0.62%.
The diff coverage is 64.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5540      +/-   ##
==========================================
+ Coverage   71.11%   71.73%   +0.62%     
==========================================
  Files         410      410              
  Lines       36310    37917    +1607     
  Branches     5585     5999     +414     
==========================================
+ Hits        25822    27201    +1379     
- Misses       8848     9026     +178     
- Partials     1640     1690      +50     
Flag Coverage Δ *Carryforward flag
nightly 71.10% <100.00%> (ø) Carriedforward from 9e375e1
unit 51.81% <48.14%> (+0.34%) ⬆️

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

Impacted Files Coverage Δ
jwst/lib/set_telescope_pointing.py 88.45% <64.28%> (+0.77%) ⬆️
jwst/pipeline/calwebb_detector1.py 71.28% <0.00%> (-20.38%) ⬇️
jwst/tso_photometry/tso_photometry_step.py 47.50% <0.00%> (-18.02%) ⬇️
jwst/white_light/white_light_step.py 61.11% <0.00%> (-15.82%) ⬇️
jwst/pipeline/calwebb_spec2.py 80.68% <0.00%> (-13.26%) ⬇️
jwst/resample/resample_spec.py 91.91% <0.00%> (-5.32%) ⬇️
jwst/lib/exposure_types.py 65.38% <0.00%> (-4.62%) ⬇️
jwst/wfs_combine/wfs_combine_step.py 97.61% <0.00%> (-2.39%) ⬇️
jwst/resample/resample.py 92.46% <0.00%> (-2.33%) ⬇️
jwst/assign_wcs/assign_wcs_step.py 78.57% <0.00%> (-1.99%) ⬇️
... and 17 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 202b32e...a49d192. Read the comment docs.

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.

Looks good to me.

I assume that the errors raised in the lower-level routines get trapped at the upper level, so that processing continues, but with default values used for the pointing (e.g. values from TARG_RA/DEC)? We don't want level 1 processing falling dead in its tracks for all the cases of bad data we have in some of the DMS test suites.

@hbushouse
Copy link
Collaborator

There's 1 flake8 error that needs fixing.

@stscirij
Copy link
Contributor Author

Looks good to me.

I assume that the errors raised in the lower-level routines get trapped at the upper level, so that processing continues, but with default values used for the pointing (e.g. values from TARG_RA/DEC)? We don't want level 1 processing falling dead in its tracks for all the cases of bad data we have in some of the DMS test suites.

Looks good to me.

I assume that the errors raised in the lower-level routines get trapped at the upper level, so that processing continues, but with default values used for the pointing (e.g. values from TARG_RA/DEC)? We don't want level 1 processing falling dead in its tracks for all the cases of bad data we have in some of the DMS test suites.

Yes, set_telescope_pointing traps errors if the engineering database can't be opened or if the telemetry values are zero, and uses the header keywords instead, but only if --allow-defaults is specified on the command line

v1_calculate exits in those 2 cases, but it isnt used to update the WCS, just to check that things are working

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.

None yet

3 participants