search

astropy / specreduce

Tools for the reduction of spectroscopic observations from Optical and NIR instruments
https://specreduce.readthedocs.io
58 stars 38 forks source link

fix for masked values in FitTrace #205

Closed cshanahan1 closed 7 months ago

cshanahan1 commented 8 months ago

This PR fixes some issues with FitTrace when bins are fully masked:

  1. If peak_method=gaussian, the fit will fail if even one column/bin is fully masked, with the error being raised by the fitter. This PR fixes that issue by setting the fit trace value to nan for that column, so it can proceed and fit the next bin. For the final fit to all bin peaks for the trace, these nans bins/columns will then be filtered.

  2. If peak_method=max or centroid, a warning when a fully masked bin is encountered claiming that it is 'Falling back on trace value from all-bin fit.'. This is not accurate to what is happening 2a. for max, the argmax of that bin will be returned which is always 0 for a fully masked bin. To avoid skewing the final fit to all bins when 0, i think a more appropriate value for this is 'nan' which can then be filtered from the final fit. theres nothing to really 'fall back' to in this case. This will require fixing almost all of the tests, so for now in this PR i just modify the warning message and retain the current behavior. See ISSUE. 2b. for centroid, when there is a fully masked bin it is also not 'falling back to the all bin fit'. to find the centroid of a fully masked bin, it attempts to interpolate between nans and will always return the maximum index in the bin. Like max, i am not changing the current behavior in this PR, that can be done as a follow up to resolve the corresponding ISSUE, but I change the warning message to reflect what is actually happening at the moment.

I also reorganized some stuff out of post_init into its own method, but this change is inconsequential.

codecov[bot] commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (b0b1b08) 81.61% compared to head (b8c65c3) 81.86%.

:exclamation: Current head b8c65c3 differs from pull request most recent head cf68b9f. Consider uploading reports for the commit cf68b9f to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #205 +/- ## ========================================== + Coverage 81.61% 81.86% +0.24% ========================================== Files 10 10 Lines 1001 1009 +8 ========================================== + Hits 817 826 +9 + Misses 184 183 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.