Spatial flexure reorganisation #1860
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## spatflex_options #1860 +/- ##
====================================================
+ Coverage 38.04% 38.05% +0.01%
====================================================
Files 211 211
Lines 49226 49252 +26
====================================================
+ Hits 18730 18745 +15
- Misses 30496 30507 +11 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
@rcooke-ast Thank you for doing this.
I tested the code a bit and it looks good. I only found a bug, which was not introduced here, but your code exposed it! And I have a few more comments/requests.
Please take a look at them, but I am approving anyway.
pypeit/calibrations.py
Outdated
| @@ -756,11 +775,12 @@ def get_flats(self): | |||
| = [], None, None, illum_setup, None, detname | |||
There was a problem hiding this comment.
raw_pixel_index should be added here too, as an empty list (just for consistency)
There was a problem hiding this comment.
Agreed - good catch
| @@ -160,7 +160,7 @@ def construct_file_name(cls, calib_key, calib_dir=None, basename=None): | |||
|
|
|||
| def buildimage_fromlist(spectrograph, det, frame_par, file_list, bias=None, bpm=None, dark=None, | |||
| scattlight=None, flatimages=None, maxiters=5, ignore_saturation=True, | |||
| slits=None, mosaic=None, calib_dir=None, setup=None, calib_id=None): | |||
| slits=None, mosaic=None, manual_spat_flexure=None, calib_dir=None, setup=None, calib_id=None): | |||
There was a problem hiding this comment.
add manual_spat_flexure description in the docstring
There was a problem hiding this comment.
Thanks, added 👍
| @@ -401,7 +401,7 @@ def build_rn2img(self, units='e-', digitization=False): | |||
| return np.array(rn2) | |||
|
|
|||
| def process(self, par, bpm=None, scattlight=None, flatimages=None, bias=None, slits=None, dark=None, | |||
| mosaic=False, debug=False): | |||
| mosaic=False, manual_spat_flexure=None, debug=False): | |||
There was a problem hiding this comment.
add manual_spat_flexure description in the docstring
Co-authored-by: Debora Pelliccia <[email protected]>
There was a problem hiding this comment.
Thanks for the review @debora-pe, and for catching that bug! All comments addressed, and merging.
pypeit/calibrations.py
Outdated
| @@ -756,11 +775,12 @@ def get_flats(self): | |||
| = [], None, None, illum_setup, None, detname | |||
There was a problem hiding this comment.
Agreed - good catch
| @@ -160,7 +160,7 @@ def construct_file_name(cls, calib_key, calib_dir=None, basename=None): | |||
|
|
|||
| def buildimage_fromlist(spectrograph, det, frame_par, file_list, bias=None, bpm=None, dark=None, | |||
| scattlight=None, flatimages=None, maxiters=5, ignore_saturation=True, | |||
| slits=None, mosaic=None, calib_dir=None, setup=None, calib_id=None): | |||
| slits=None, mosaic=None, manual_spat_flexure=None, calib_dir=None, setup=None, calib_id=None): | |||
There was a problem hiding this comment.
Thanks, added 👍
| @@ -401,7 +401,7 @@ def build_rn2img(self, units='e-', digitization=False): | |||
| return np.array(rn2) | |||
|
|
|||
| def process(self, par, bpm=None, scattlight=None, flatimages=None, bias=None, slits=None, dark=None, | |||
| mosaic=False, debug=False): | |||
| mosaic=False, manual_spat_flexure=None, debug=False): | |||
This delta PR moves the manual spatial flexure correction to the
RawImage. The benefit of this approach is that manual spatial flexure can now be assigned for any frame (including calibrations). Previously, it was only science/standard/background frames that could have a manual flexure. Tests pass: