Skip to content

Add an array-API median fallback for subtract_overscan - #989

Merged
mwcraig merged 4 commits into
astropy:mainfrom
mwcraig:fix-median-fallback-overscan
Aug 25, 2026
Merged

Add an array-API median fallback for subtract_overscan#989
mwcraig merged 4 commits into
astropy:mainfrom
mwcraig:fix-median-fallback-overscan

Conversation

@mwcraig

@mwcraig mwcraig commented Aug 24, 2026

Copy link
Copy Markdown
Member

What

subtract_overscan calls xp.median(overscan.data, axis=overscan_axis)
(ccdproc/core.py), but median is not part of the array API standard, so
it raises AttributeError on backends such as array-api-strict that don't
provide it (numpy, jax and dask all have a native median and are
unaffected).

This adds ccdproc._nanfuncs.median, a small wrapper around the existing
nanmedian fallback, and uses it from a new _median_fallback helper in
core.py that tries the namespace's own median first and only falls back
to the array-API-only implementation when it's missing -- mirroring the
existing _percentile_fallback pattern already used for percentile.

Why

Part of #971 (array-API migration tracker). median joins nansum /
nanmean / nanstd / nanmedian (#906, #986) as another reduction that
isn't part of the array API standard and needs a pure-array-API fallback.

NaN semantics

numpy.median propagates NaN: if any value in the reduced slice is NaN, the
result is NaN. nanmedian does the opposite -- it ignores NaNs. Since
subtract_overscan's median fallback needs to match numpy.median
(not nanmedian), the new median function wraps nanmedian and then
restores NaN-propagation with a final xp.where(xp.any(xp.isnan(x), ...)).
On NaN-free input median is bit-for-bit identical to nanmedian, since it
reuses the exact same sort-and-average algorithm.

Testing

  • ccdproc/tests/test_nanfuncs.py: added median (against np.median) to
    the differential _FUNCS table, which already exercises NaN-scattered,
    all-NaN, single-non-NaN, integer and boolean inputs across a range of
    shapes and axes -- this is what pins the NaN-propagating semantics. Also
    added median to test_no_warning_on_all_nan_slice and test_bad_axis
    (verified numpy.median, unlike numpy.nanmedian, does not warn on
    all-NaN input).
  • array-api-strict: all "module 'array_api_strict' has no attribute
    'median'" errors are eliminated -- confirmed zero occurrences anywhere in
    the suite after this change.
  • numpy backend: ccdproc suite is unaffected (0 failures, native xp.median
    path taken, byte-identical to before).
  • dask backend: test_nanfuncs.py + test_ccdproc.py pass.

Note on the strict-suite failure count: the 4 tests originally cited as
failing due to this bug (test_ccd_process[*],
test_ccd_process_gain_corrected) do not go green with this fix alone.
Diffing the full array-api-strict failure list before/after shows the same
85 tests fail both times -- but for 3 of these 4 tests the reason changes,
from AttributeError: ... has no attribute 'median' to two separate,
pre-existing bugs that this fix now uncovers (they were previously masked
because subtract_overscan raised first):

  • ccd_process's bad-pixel-mask handling uses xp.asarray(bad_pixel_mask, dtype=bool) with the Python builtin bool instead of the namespace's
    xp.bool (core.py, in ccd_process), which array-api-strict rejects.
  • gain_correct builds its gain array with xp.asarray(gain_value) without
    specifying device=, so it ends up on the default device while the image
    data is on array-api-strict's non-default device, and the subsequent
    multiply fails with "Arrays from two different devices".

Both are real, independent array-API incompatibilities, out of scope for
this PR, and worth filing as separate follow-up issues against #971.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RQMJZUaaxfqGDk41GLSaFK

mwcraig and others added 2 commits August 24, 2026 10:19
subtract_overscan called xp.median(overscan.data, axis=overscan_axis),
but median is not part of the array API standard, so it raised
AttributeError on backends such as array-api-strict that omit it
(numpy, jax and dask all provide it natively and are unaffected).

Add ccdproc._nanfuncs.median, a NaN-propagating wrapper around the
existing nanmedian fallback: on NaN-free input it is bit-for-bit
identical to nanmedian, and a final where() restores numpy.median's
NaN-propagating semantics (nanmedian alone would silently ignore
NaNs). subtract_overscan now goes through a new _median_fallback
helper in core.py, mirroring the existing _percentile_fallback
pattern: try the namespace's own median first, and only fall back to
the array-API-only implementation when it is missing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RQMJZUaaxfqGDk41GLSaFK
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RQMJZUaaxfqGDk41GLSaFK
@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.44%. Comparing base (3e644d8) to head (5399fc2).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #989      +/-   ##
==========================================
+ Coverage   97.42%   97.44%   +0.01%     
==========================================
  Files           9        9              
  Lines        1749     1760      +11     
==========================================
+ Hits         1704     1715      +11     
  Misses         45       45              
Flag Coverage Δ
dask 96.57% <100.00%> (+0.02%) ⬆️
jax 96.63% <100.00%> (+0.02%) ⬆️
numpy 97.32% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

codecov flagged the two lines of _median_fallback's AttributeError branch
(core.py:182,186) as the only uncovered lines in the patch: every backend
that uploads coverage (numpy, jax, dask) has a native median, so the
fallback only ran on the strict job, which reports no coverage. Exercise
it everywhere with a proxy namespace that hides median and otherwise
delegates to the backend under test, following the fake-namespace
pattern in test_combiner.py.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RQMJZUaaxfqGDk41GLSaFK
Comment thread ccdproc/_nanfuncs.py Outdated
Review feedback on astropy#989: the paragraph explaining how median relates to
nanmedian is implementation detail, and the other functions in this
module keep that material under Notes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RQMJZUaaxfqGDk41GLSaFK
@mwcraig
mwcraig merged commit 1b9b621 into astropy:main Aug 25, 2026
15 of 16 checks passed
@mwcraig
mwcraig deleted the fix-median-fallback-overscan branch August 25, 2026 13:37
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