-
-
Notifications
You must be signed in to change notification settings - Fork 41
Image data handling refactor #228
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
base: main
Are you sure you want to change the base?
Conversation
…e trace . . . .. , . . code style .
…c method and modified the method's logic accordingly.
…nd mask_treatment.
…nd mask_treatment.
…ns' instead of 'pytest.raises' to test for an expected warning.
…duce.core._ImageParser' mixin class.
Renamed the `Image` class to `SRImage` for better clarity and added comprehensive validation for the dispersion and cross-dispersion axes. Introduced properties to provide reordered views of data, mask, and uncertainty arrays.
Redesigned the SRImage class to centralise image initialisation and sanity checks. Standardised axis handling across the class.
Replaced `_ImageParser` with `SRImage` in `Background` class to standardize image handling. Added utility functions in `SRImage` for masking and subtraction, and updated related methods and tests to accommodate these changes.
…RImage` class to simplify dimension handling. Refactored trace calculations in `tracing.py` to utilize these new properties.
… replacing the _ImageParser. Updated related tests and modules to accommodate SRImage-specific methods and streamlined code a bit.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #228 +/- ##
===========================================
- Coverage 83.37% 26.72% -56.66%
===========================================
Files 13 14 +1
Lines 1137 1220 +83
===========================================
- Hits 948 326 -622
- Misses 189 894 +705 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
The commit history of this PR is dirty. Please rebase. |
This draft PR proposes one way to refactor image data handling in Specreduce. The PR introduces a
specreduce.image.SRImageclass that works as a (relatively thin) wrapper aroundastropy.nddata.NDDataRef. I chose to wrapNDDataRefinstead ofspecutils.Spectrum1Dbecause I think this is a more appropriate class for 2D images, especially when we start working on 2D wavelength calibration. The methods that produce 1D spectra still returnSpectrum1Dobjects.The main functionalities of
SRImageare:SRImageinitialiser combines the image parsing logic that was previously inspecreduce.core._ImageParserandspecreduce.extract.HorneExtract._parse_image.numpy.moveaxisinstead ofnumpy.transposein the case we want to support images withndim > 2in the future. (The class hascrossdisp_axisanddisp_axisproperties, which are fixed to 0 and 1, respectively.)apply_maskmethod that applies the mask to the image data using the logic introduced by @cshanahan1 in PR Add default and set to zero masking option for specreduce operations. #216, and ato_masked_arraymethod that applies the mask and returns the data as a masked array.The PR modifies the
Background,Tracing, andExtractionclasses to work with theSRImageclass. The refactor passes all the tests without modifications to the tests themselves (well, except the ones forHorneExtract, which is something I still need to work on), so the changes should not modify or break the functionality of any existing code using public methods.While it passes (nearly) all the tests, this PR is not ready to be merged. I've created it to raise discussion and hear everyone's opinion about the approach to the refactor. So, please comment on whether this is a sensible way to go or if you'd prefer something else.
If this approach gains sufficient support, I'll polish the code and prepare the PR ready to be merged. In any case, this will need to wait until PR #216 is merged (the code is based on the masking work by @cshanahan1 in #216), and I still need to document the code properly and rewrite some of the
HorneExtracttests to work with the new image handling logic, but my aim is that theHorneExtractionclass itself will work identically as before without any visible changes to the end-user.