Enable resample_spec for NIRSpec line lamp exposures #5484
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5484 +/- ##
==========================================
+ Coverage 71.13% 71.51% +0.38%
==========================================
Files 410 410
Lines 36278 37392 +1114
Branches 5883 6212 +329
==========================================
+ Hits 25805 26741 +936
- Misses 8833 8983 +150
- Partials 1640 1668 +28
*This pull request uses carry forward flags. Click here to find out more.
Continue to review full report at Codecov.
|
jwst/resample/resample_spec.py
Outdated
| x_tan, y_tan = undist2sky1.inverse(ra, dec) | ||
| warnings.resetwarnings() | ||
| if model.meta.wcs.output_frame.name == 'world': | ||
| # ra and dec this converted to tangent projection |
There was a problem hiding this comment.
The grammar in this comment could use some cleanup. What does "ra and dec this converted" really mean?
There was a problem hiding this comment.
This is a holdover from before my changes, I'll clean up
stscirij
force-pushed
the
lamps_resample_spec
branch
from
8ac17f0
to
0354440
Compare
| # at this center of slit find x,y tangent projection - x_tan, y_tan | ||
| x_tan, y_tan = undist2sky1.inverse(ra, dec) | ||
| warnings.resetwarnings() | ||
| else: |
There was a problem hiding this comment.
I wonder if this else statement is too general. If the general idea is to have coordinates on a plane and If the output_frame is V2V3 the code may not work correctly since V2V3 is a spherical system. I'm not sure if it's better to exclude V2V3 or to consider only msa_frame. The first option allows for other frames to work so it may be prefferable.
There was a problem hiding this comment.
It's a tricky one - currently resample_spec only 'knows' how to do sky/world and msa_frame, so we could just test those cases and punt if there's a different output frame. Alternatively, maybe we could do something like
if Unit("deg") in model.meta.wcs.output_frame.unit or Unit("arcsec") in model.meta.wcs.output_frame.unit:
to capture the sky-like cases, and otherwise do the cartesian cases
jwst/resample/resample_spec.py
Outdated
| x_tan_all, y_tan_all = undist2sky.inverse(all_ra, all_dec) | ||
| if model.meta.wcs.output_frame.name == 'world': | ||
| # find the spatial size of the output - same in x,y | ||
| x_tan_all, y_tan_all = undist2sky.inverse(all_ra, all_dec) |
There was a problem hiding this comment.
The definition of undist2sky can go in the if statement.
jwst/resample/resample_spec.py
Outdated
| sky = cf.CelestialFrame(name='sky', axes_order=(0, 1), | ||
| reference_frame=coord.ICRS()) | ||
| else: | ||
| sky = cf.Frame2D(name='sky', axes_order=(0, 1)) |
There was a problem hiding this comment.
The name is a bit misleading since it's definitely not sky. Is something like
resampled_+ {model.meta.wcs.output_frame.name} better?
There was a problem hiding this comment.
Yes, I thought the naming was only temporary for the purposes of doing the transformation, but now I see that it appears in the meta.wcs.output_frame of the slits in the _s2d file.
frames cleaner by tying the test to output frame units
Addresses JP-1733
Fixes #5366