Skip to content

Add a spec-only nanmedian fallback for Combiner.median_combine - #978

Open
mwcraig wants to merge 2 commits into
astropy:mainfrom
mwcraig:fix-906-nanmedian-fallback
Open

Add a spec-only nanmedian fallback for Combiner.median_combine#978
mwcraig wants to merge 2 commits into
astropy:mainfrom
mwcraig:fix-906-nanmedian-fallback

Conversation

@mwcraig

@mwcraig mwcraig commented Aug 22, 2026

Copy link
Copy Markdown
Member

Adds ccdproc/_nanmedian.py, a NaN-aware median written only in terms of array-API-standard functions (sort the axis, count non-NaN entries, gather and average the two middle elements; all-NaN slices give NaN; integer input is promoted to float64). Combiner._default_median now returns this fallback (bound to the namespace via functools.partial) when the namespace has no nanmedian, instead of raising RuntimeError. The fallback is O(n log n) along the combination axis; native nanmedian / bottleneck are still preferred when available.

Also:

  • docs/array_api.rst: replace the claim of a bottleneck -> numpy fallback (which did not exist) with the real chain (bottleneck for numpy -> xp.nanmedian -> spec-only fallback).
  • test_combiner.py: drop the numpy-median/.compute() workarounds and the pytest.skip for namespaces without median; test_bottleneck_defaults_respect_array_namespace now checks the fallback is returned when nanmedian is absent.
  • New ccdproc/tests/test_nanmedian.py (odd/even lengths, 1-D/2-D/3-D, partial and all-NaN columns, integer input, axis=1/-1, unsupported axis).

Test matrix (pytest ccdproc):

  • numpy: 393 passed, 5 skipped
  • jax: 379 passed, 10 skipped, 2 xfailed, 7 xpassed (the 7 XPASS are identical on upstream/main; astropy/jax DeprecationWarning markers, unrelated)
  • dask (with CCDPROC_LOG_ARRAY_ESCAPES=1 CCDPROC_ENFORCE_ESCAPE_BASELINE=1): 383 passed, 15 skipped; no escapes outside the baseline
  • array-api-strict, test_nanmedian.py: 13 passed
  • array-api-strict, test_combiner.py: 75 failed / 12 passed before -> 74 failed / 13 passed after. All remaining median_combine failures under strict hit the Combiner constructor ("Nested Arrays are not allowed", being fixed in Build Combiner data and mask arrays with xp.stack #976) or other known strict bugs, not the median itself.

Fixes #906

🤖 Generated with Claude Code

https://claude.ai/code/session_01DTN9DnPnLKK2u7knnMJ2gA

Add ccdproc/_nanmedian.py, a NaN-aware median written purely in terms of
the array API standard (sort, count non-NaN, gather the two middle
elements), and use it from _default_median when the selected namespace
has no nanmedian instead of raising RuntimeError. Works on numpy, jax,
dask and array-api-strict.

Fix the docs, which claimed a bottleneck -> numpy fallback that did not
exist, and drop the numpy-median and pytest.skip workarounds in
test_combiner.py now that a median is available on every backend.

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

codecov Bot commented Aug 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.84%. Comparing base (89e90f9) to head (7c96d93).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
ccdproc/combiner.py 50.00% 2 Missing ⚠️
ccdproc/_nanmedian.py 96.15% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #978      +/-   ##
==========================================
+ Coverage   95.83%   95.84%   +0.01%     
==========================================
  Files           8        9       +1     
  Lines        1631     1659      +28     
==========================================
+ Hits         1563     1590      +27     
- Misses         68       69       +1     

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

pre-commit.ci has been failing on main since the black 26.5.1 bump in
astropy#919 because black now requires a blank line after the module docstring
in ccdproc/__init__.py. This is the only formatting change it wants.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DTN9DnPnLKK2u7knnMJ2gA
@mwcraig
mwcraig marked this pull request as ready for review August 22, 2026 18:24
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.

Consider adding fallback median implementation

1 participant