Skip to content

WavelengthSolution1D.resample bug fixes and improvements - #319

Open
hpparvi wants to merge 4 commits into
astropy:mainfrom
hpparvi:v110_wavesol1d_improvements
Open

WavelengthSolution1D.resample bug fixes and improvements#319
hpparvi wants to merge 4 commits into
astropy:mainfrom
hpparvi:v110_wavesol1d_improvements

Conversation

@hpparvi

@hpparvi hpparvi commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

This PR fixes several bugs in the WavelengthSolution1D.resample method and improves the method's overall functionality.

Bug fixes

  • The flux was multiplied by the pixel wavelength width before dividing by the bin width, so the output was labeled as a flux density but was off by the local dispersion and did not conserve the integrated flux. The density and its uncertainty are now computed solely from the fractional pixel overlaps, with the variance using the squared weights. (I somehow remembered I had fixed this a long time ago, so I'm not sure what happened here...)
  • Spectra whose spectral axis does not start at pixel zero were indexed from the wrong pixels.
  • Solutions where the wavelength decreases with pixel number silently produced zero flux. The output is now always ascending in wavelength.

Changes

  • The input mask is propagated (a bin is flagged if any contributing pixel is masked) and the metadata is copied.
  • A spectrum without uncertainty returns no uncertainty instead of a fabricated zero-valued one.
  • The output spectral axis carries the bin edges actually used, so spectral_axis.bin_edges is exact for non-uniform grids and integrating the flux over those edges recovers the input counts exactly.

AI and LLM use disclaimer: I'm systematically going through the specreduce codebase with Claude Code (Fable 5.1) to identify bugs, issues, and any room for (non-breaking) improvements in the existing functionality before the specreduce 2.10 release. The first, rather serious, bug I was aware of beforehand, but I remembered I had it already fixed several months ago. I also used Claude Code to implement most of the changes. I understand the code and the changes and could have implemented them myself, but it would have taken significantly longer than the 2-3 hours of continuous work it took to create this PR.

…multiplying the per-pixel flux by the pixel wavelength width before dividing by the bin width. The output was labelled as a flux density but was numerically off by the local dispersion and did not conserve the integrated flux. I really thought I fixed this months ago already...
…is does not start at pixel zero, and for solutions where the wavelength decreases with pixel number (the output is now always ascending in wavelength).

- Changed `WavelengthSolution1D.resample` to propagate the input mask, copy the input metadata, and return no uncertainty instead of a fabricated zero-valued one when the input has none.
…ally used on the output spectral axis, so spectral_axis.bin_edges is exact for non-uniform bin grids instead of being inferred from the bin centres.
@codecov

codecov Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.54%. Comparing base (742264f) to head (4c56a82).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #319      +/-   ##
==========================================
+ Coverage   92.52%   92.54%   +0.02%     
==========================================
  Files          18       18              
  Lines        2341     2348       +7     
==========================================
+ Hits         2166     2173       +7     
  Misses        175      175              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant