8000 JP-488 and JP-1920: NIRSpec `bounding_box` is sometimes too large. by WilliamJamieson · Pull Request #6257 · spacetelescope/jwst · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

JP-488 and JP-1920: NIRSpec bounding_box is sometimes too large. #6257

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Aug 4, 2021
Merged

JP-488 and JP-1920: NIRSpec bounding_box is sometimes too large. #6257

merged 10 commits into from
Aug 4, 2021

Conversation

WilliamJamieson
Copy link
Collaborator
@WilliamJamieson WilliamJamieson commented Jul 30, 2021 •
edited by hbushouse
Loading

Closes #5736
Closes #2958
Resolves JP-488 and JP-1920

Description
The bounding_box for NIRSpec is sometimes significantly oversized in height (y direction). This was because the ~jwst.assign_wcs.compute_bounding_box was very naive in how it computed the range of acceptable y values. The changes here take the bounding box "guessed" by this naive method and evaluate the transform, and then finds the exact bounds of where the spectral trace actually lands (the non-NaN values). These bounds are then used to compute an improved bounding box.

Checklist

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

@WilliamJamieson WilliamJamieson requested a review from nden July 30, 2021 19:59
@codecov
Copy link
codecov bot commented Jul 30, 2021 •
edited
Loading

Codecov Report

Merging #6257 (1c94d55) into master (7a8738b) will decrease coverage by 0.93%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6257      +/-   ##
==========================================
- Coverage   76.68%   75.74%   -0.94%     
==========================================
  Files         405      405              
  Lines       35330    37483    +2153     
==========================================
+ Hits        27093    28393    +1300     
- Misses       8237     9090     +853
Flag Coverage Δ *Carryforward flag
nightly 77.81% <100.00%> (ø) Carriedforward from fe0d202
unit 56.89% <100.00%> (+0.11%) ⬆️

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

Impacted Files Coverage Δ
jwst/assign_wcs/nirspec.py 94.46% <100.00%> (+0.43%) ⬆️
jwst/lib/v1_calculate.py 60.81% <0.00%> (-39.19%) ⬇️
jwst/tweakreg/tweakreg_step.py 47.88% <0.00%> (-19.48%) ⬇️
jwst/extract_1d/extract.py 49.50% <0.00%> (-15.91%) ⬇️
jwst/lib/set_telescope_pointing.py 87.89% <0.00%> (-1.63%) ⬇️
jwst/cube_build/blot_cube_build.py 74.90% <0.00%> (-1.20%) ⬇️
jwst/resample/resample.py 96.36% <0.00%> (-0.42%) ⬇️
jwst/jump/jump.py 100.00% <0.00%> (ø)
jwst/lib/set_velocity_aberration.py 100.00% <0.00%> (ø)
jwst/cube_build/blot_cube.py
... and 3 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 7a8738b...1c94d55. Read the comment docs.

@nden nden added this to the Build 7.9 milestone Aug 2, 2021
nden
nden approved these changes Aug 4, 2021
View reviewed changes
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.

Looks good. I left some minor comments, mostly to clean and clarify the code.

WilliamJamieson and others added 2 commits August 4, 2021 09:44
@WilliamJamieson @nden
Apply suggestions from code review
aeaba7e 
Co-authored-by: Nadia Dencheva <nadia.astropy@gmail.com>
@WilliamJamieson
Removed commented lines.
1c94d55
@nden nden merged commit cbfc7da into spacetelescope:master Aug 4, 2021
@stscijgbot stscijgbot mentioned this pull request Aug 4, 2021
Closed
@WilliamJamieson WilliamJamieson deleted the tickets/JP-488 branch August 4, 2021 17:36
@stscijgbot-jp stscijgbot-jp mentioned this pull request Aug 11, 2021
Closed
loicalbert pushed a commit to talensgj/jwst that referenced this pull request Nov 5, 2021
@WilliamJamieson @nden @loicalbert
JP-488 and JP-1920: NIRSpec bounding_box is sometimes too large. (s…
97a98d1 
…pacetelescope#6257)

* Using, `slit2detector.inverse` to narrow `bounding_box` range.

* Updated inverse call to fit actual inverse interface.

* Tried to solve the inverse issues.

* Brute force y_range

* Added padding to y coords

* Fixed bug when guessed bounding_box ranges are bad.

* Fixed broken test

* Updated change log.

* Apply suggestions from code review

Co-authored-by: Nadia Dencheva <nadia.astropy@gmail.com>

* Removed commented lines.

Co-authored-by: Nadia Dencheva <nadia.astropy@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nden nden nden approved these changes

Assignees
No one assigned
Labels
assign_wcs NIRSPEC
Projects
None yet
Milestone
Build 7.9
Development

Successfully merging this pull request may close these issues.

NIRSpec MOS extract_2d bounding boxes too large in some cases NIRSpec FS extract_2d product for S200B1 is oversized
3 participants
@WilliamJamieson @nden @hbushouse
0