8000 JP-2802: Fix a bug in the definition of NIRSpec resampled WCS by mcara · Pull Request #7359 · spacetelescope/jwst · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

JP-2802: Fix a bug in the definition of NIRSpec resampled WCS #7359

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

Merged
merged 2 commits into from
Nov 30, 2022

Conversation

mcara
Copy link
Member
@mcara mcara commented Nov 17, 2022

Resolves JP-2802
Closes #7265

This PR fixes a bug in the definition of the NIRSpec resampled WCS. Fundamentally, the bug was that for the forward transformation the horizontal position in the slit was incorrect. This PR fixes horizontal position to 0 - center of the slit. I expect regression tests to fail.

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
  • Make sure the JIRA ticket is resolved properly

Regression test: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/469/

@mcara mcara added this to the Build 9.1 milestone Nov 17, 2022
@mcara mcara requested review from nden and hbushouse November 17, 2022 19:31
@mcara mcara self-assigned this Nov 17, 2022
@github-actions github-actions bot added resample and removed NIRSPEC labels Nov 17, 2022
@codecov
Copy link
codecov bot commented Nov 17, 2022

Codecov Report

Base: 79.64% // Head: 79.57% // Decreases project coverage by -0.06% ⚠️

Coverage data is based on head (c474f19) compared to base (40c70fc).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7359      +/-   ##
==========================================
- Coverage   79.64%   79.57%   -0.07%     
==========================================
  Files         412      412              
  Lines       37572    37730     +158     
==========================================
+ Hits        29924    30025     +101     
- Misses       7648     7705      +57     
Flag Coverage Δ *Carryforward flag
nightly 79.62% <100.00%> (ø) Carriedforward from 40c70fc
unit 52.23% <100.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
jwst/resample/resample_spec.py 74.59% <100.00%> (-3.70%) ⬇️

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.

@mcara mcara force-pushed the fix-nirspec-resampled-wcs branch from e2bb2cc to 38bdbcf Compare November 21, 2022 12:49
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. Just need to remove a print statement that shouldn't be there.

@mcara mcara force-pushed the fix-nirspec-resampled-wcs branch from 38bdbcf to e508f9e Compare November 22, 2022 14:48
@github-actions github-actions bot removed the NIRSPEC label Nov 22, 2022
@nden nden force-pushed the fix-nirspec-resampled-wcs branch from e508f9e to 43208ed Compare November 28, 2022 20:55
Copy link
Collaborator
@nden nden 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

The CI failure appears to be unrelated, so I'm going to merge.

@hbushouse hbushouse merged commit d5d4086 into spacetelescope:master Nov 30, 2022
@mcara mcara deleted the fix-nirspec-resampled-wcs branch April 8, 2023 16:59
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.

Weird S_REGION for Spec2Pipeline resample_spec for 01539003001
3 participants
0