Array-API-native block_reduce, block_average and block_replicate off numpy by mwcraig · Pull Request #1009 · astropy/ccdproc

mwcraig · 2026-09-07T20:46:38Z

ccdproc.core.block_reduce, block_average and block_replicate are thin
wrappers around astropy.nddata, which starts by calling numpy.asanyarray
on its input. On a non-numpy array library that silently hands back a numpy
array (dask, jax) or fails outright when the data live on a device numpy
cannot reach (array-api-strict, and by extension cupy on a GPU). These were
three of the remaining array-API escapes in the escape baseline.

This adds ccdproc/_blocks.py, which does the same work using only array API
operations (reshape, permute_dims, repeat and slicing), and dispatches
to it for every namespace that is not numpy.

This is deliberately a stopgap: it exists so ccdproc's array-API work does not
have to wait for an astropy release. _blocks.block_reduce and
_blocks.block_replicate, minus the ccdproc dispatch, are the intended
starting point for astropy/astropy#15073; once astropy's own blocks.py is
array-API aware and that version is ccdproc's floor, this module and the
dispatch both go away. (block_average has no astropy counterpart -- it is
ccdproc's own thin wrapper -- so only the other two are candidates for the
lift.)

Dispatch policy

The numpy path is unchanged: if array_api_compat.is_numpy_namespace(xp) the
functions call astropy.nddata exactly as before, with the same arguments.
Everything else goes to _blocks. The CCDData rebuild and the
"following attributes were set ... will be ignored" warning behaviour are
untouched -- the native helpers are decorated with
astropy.nddata.support_nddata, as astropy's own are, so a CCDData
argument is unpacked and warned about identically on every backend.

block_size validation is done in pure Python and reproduces astropy's three
checks in astropy's order with astropy's messages verbatim; the tests assert
against astropy's live output rather than hard-coded strings so the two cannot
drift apart.

Behaviour differences

Results are identical to astropy's except for two dtype promotions, both of
integer/boolean input to the namespace's default real floating dtype:

block_replicate(..., conserve_sum=True) promotes before dividing, because
array-api-strict rejects integer true division rather than promoting.
block_average promotes before averaging, because array-api-strict rejects
a non-floating mean. numpy promotes on its own here, and jax and dask
follow it, so without this an integer image averaged fine on three backends
and raised on the fourth.

numpy returns float64 in both cases anyway, so these differ only for a
library whose default real dtype is not float64. Documented in
docs/array_api.rst and CHANGES.rst.

Verified backends

Run with the dev environment directly, CCDPROC_ARRAY_LIBRARY=<lib> (plus
JAX_ENABLE_X64=1 for jax); array-api-strict runs on its non-default
device1, which is where np.asarray raises.

pytest ccdproc/tests/test_blocks.py ccdproc/tests/test_ccdproc.py:

backend	result
numpy	168 passed
jax	168 passed
dask	168 passed
array-api-strict (device1)	159 passed, 9 xfailed

Full suite, this branch vs. the merge base:

backend	branch	merge base
numpy	1045 passed, 5 skipped	975 passed, 5 skipped
array-api-strict	997 passed, 10 skipped, 43 xfailed	924 passed, 10 skipped, 46 xfailed
dask (escape enforcement)	1034 passed, 16 skipped	not run
jax	1040 passed, 10 skipped	not run

The deltas are fully accounted for: the 70 new tests in test_blocks.py, plus
the three formerly-xfailed array-api-strict tests now passing.

The three block_* lines are removed from
ccdproc/tests/array_escape_baseline.txt. Verified with a full dask run under
CCDPROC_LOG_ARRAY_ESCAPES=1 CCDPROC_ENFORCE_ESCAPE_BASELINE=1: no escapes
outside the baseline, no block_* sites in the escape log, and temporarily
restoring the three lines makes the ratchet report exactly those three as
"not hit this run" and nothing else.

Things worth a reviewer's eye

_nanfuncs._fill_doc gained a positional-only template argument so
_blocks.py can reuse the docstring templating with its own parameter
block. Backwards compatible; no _nanfuncs docstring changed.
core._block_namespace resolves the namespace as
ccd.data if isinstance(ccd, CCDData) else ccd, matching sigma_func's
convention. The old code used ccd.data unconditionally, which crashed for
a bare array (np.ndarray.data is a memoryview) -- bare arrays now work.
The narrowing is that a plain list passed with an explicit func used to
reach astropy and now raises TypeError from array_namespace; nothing in
the test suite or docs does that.
The promotion guard is xp.isdtype(dtype, ("integral", "bool")) rather than
"not real floating", so complex input is not silently truncated to real.
With the array-api-strict xfail markers removed, the three existing block
tests failed one step later on their own xp.zeros((4, 4), dtype=bool),
which array-api-strict rejects; they now ask for dtype=xp.bool, as the
rest of that file already does. The ccd._mask = ... TODO is untouched.
The first commit is green only on numpy: test_blocks.py ships with it and
includes two tests of the core dispatch that need the second commit on
non-numpy backends. Squashing the first two commits would fix that if you
care about per-commit bisection on the backend jobs.

Part of #971.

🤖 Generated with Claude Code

https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN

codecov · 2026-09-07T21:00:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.15%. Comparing base (9d18599) to head (2f866f7).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1009      +/-   ##
==========================================
+ Coverage   97.97%   98.15%   +0.18%     
==========================================
  Files           9       10       +1     
  Lines        1927     2007      +80     
==========================================
+ Hits         1888     1970      +82     
+ Misses         39       37       -2

Flag	Coverage Δ
dask	`97.40% <100.00%> (+0.21%)`	⬆️
jax	`97.45% <98.13%> (+0.10%)`	⬆️
numpy	`97.90% <97.19%> (+0.03%)`	⬆️

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 two untested branches in astropy#1009: the early return in _block_namespace when the caller passes xp, and the int() failure branch in _block_size that turns a NaN or infinite block size into astropy's "must be integers" error. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN

Copilot

🟡 Changes recommended

Namespace inference introduces a breaking regression for previously supported array-like inputs.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds array-API-native block operations while retaining Astropy for NumPy inputs.

Changes:

Implements native reduction, averaging, and replication.
Adds namespace/device preservation tests and documentation.
Removes resolved array-escape baselines.

File summaries

File	Description
`ccdproc/_blocks.py`	Implements native block operations.
`ccdproc/core.py`	Dispatches by array namespace.
`ccdproc/_nanfuncs.py`	Generalizes docstring templating.
`ccdproc/tests/test_blocks.py`	Adds differential backend tests.
`ccdproc/tests/test_ccdproc.py`	Enables strict-backend wrapper tests.
`ccdproc/tests/array_escape_baseline.txt`	Removes resolved escapes.
`docs/array_api.rst`	Documents backend behavior.
`CHANGES.rst`	Records the feature.

Review details

Files reviewed: 8/8 changed files
Comments generated: 1
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

mwcraig

Adversarial review of 34721db, focused on (a) bugs that would change scientific results or break existing callers and (b) ways to reduce complexity and line count. Every correctness claim below was reproduced by running the branch code in the strict, py312-alldeps-jax and py312-alldeps-dask tox environments; every simplification was checked against the full test_blocks.py suite on those backends.

Fix before merge (results change or callers break):

_block_namespace rejects plain NDData and nested-list input on every backend, including numpy, where main accepts them.
block_reduce on a bool mask raises on array-api-strict and returns int32 on jax, vs int64 counts from astropy.
block_replicate(conserve_sum=True) returns float32 on jax/dask/strict but float64 on numpy for float32 input, contrary to the "identical results" claim in CHANGES and the docs.
_fill_doc hard-codes a 4-space indent and produces ragged docstrings on Python 3.13 (pre-existing, but this PR adds three consumers).

Cheap and verified simplifications: the block_replicate conserve-sum branch and repeat loop, the _block_size integrality loop, and a shared dispatcher for the three core.py wrappers.

Judgment calls: inline docstrings instead of _fill_doc templating in _blocks.py, test dedup in test_blocks.py, and fusing block_average's promotion into the reduction.

Dropped after verification: the jax int32 overflow on large-block integer sums is pre-existing on main and a documented spec property (folded into the doc note on item 3).

🤖 Generated with Claude Code

https://claude.ai/code/session_017kEJ4Z2wTtZC4TzCQe8DQq

…, dedup Production code: - Rename ``_block_namespace`` to ``_namespace_for``, a generic resolver that unwraps any ``NDData`` and falls back to the numpy namespace when no array library claims the input, so plain ``NDData`` and nested-list input reach ``astropy.nddata`` exactly as on ``main``. - Add ``_block_dispatch`` so the three ``block_*`` wrappers share the numpy/native branch and the ``CCDData`` re-wrap. - ``block_reduce`` with the default reduction casts boolean input to the namespace's default integral dtype, matching astropy's bool sum instead of failing on array-api-strict or narrowing on jax. - ``block_replicate`` divides the input once, in one statement, and replicates with reshape/broadcast_to/reshape; it keeps a floating input's dtype, now documented as the second difference from astropy. - ``_block_size`` integrality check reduced to ``float(s).is_integer()``. - Inline the three Parameters sections and drop the ``_fill_doc`` generalisation, so ``_nanfuncs.py`` is unchanged from ``main``. - Docstrings: short summaries, rationale under ``Notes``. Tests: NDData/list regression tests, bool and float32 parity cases, a ``CCDData`` re-wrap test, ``match=`` substrings instead of byte-equal error messages, and the two namespace/device tests merged. Docs and changelog shortened; the jax int32 default sum width is noted. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017kEJ4Z2wTtZC4TzCQe8DQq

mwcraig · 2026-09-10T15:52:33Z

Pushed dea7711 with all agreed fixes from the review: generic _namespace_for resolver with NDData/list fallback, _block_dispatch, bool sums promoted to the default integral dtype, single-statement block_replicate division with float32 preserved, float(s).is_integer() block-size check, inlined Parameters sections with _nanfuncs.py back to main, docstring rationale under Notes, match= error tests, merged namespace/device tests, shortened CHANGES. Block tests pass on numpy, jax, dask and array-api-strict; the full suite passes on numpy and strict locally.

Follow-ups: #1015 opened for the _to_xp helper; the astropy float32 issue is drafted for review, not posted.

— Written by Claude at @mwcraig's direction.

Copilot

🟡 Changes recommended

Complex inputs currently lose their imaginary components in averaging and sum-conserving replication.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

ccdproc/_blocks.py:293

Complex input is also passed through _promote_to_real here, which converts it to a real dtype and silently loses the imaginary component before replication. Promote only integer/boolean data, then divide complex and real floating data without casting.

        data = _promote_to_real(data, xp, array_api_compat.device(data)) / math.prod(
            sizes
        )

Files reviewed: 7/7 changed files
Comments generated: 1
Review effort level: Balanced

mwcraig

Final adversarial review of dea7711, focused on science-affecting bugs and on reducing complexity. Every finding was checked by running the code on numpy, jax, dask and array-api-strict; a 200-case random shape/block-size parity sweep against astropy.nddata found no value or shape mismatch on any backend. Nothing here blocks the PR.

Science-affecting

Complex input loses its imaginary part on the native path (inline comment on _blocks.py). One-line fix in _promote_to_real, or a local guard.

Lesser correctness notes, fine to leave

An explicit xp that does not match the data crashes with an internal AttributeError (inline on core.py).
Bool input with func=xp.sum spelled out raises on strict only (inline on _blocks.py).

Complexity reductions, recommended (inline comments): fold _namespace_for into _block_dispatch; drop the reuse paragraph in its Notes; delete test_core_wrappers_rewrap_ccddata; move the trimming sentence in block_reduce under Notes; cite astropy/astropy#20360 in the docs and CHANGES.

Considered and declined

permute_dims before the reduction materialises a full-size copy on eager jax (about 35-47% of the call on a 4096x4096 float32 array). Avoiding it means branching on func is None or changing the trailing-block-axes contract astropy gives user functions. Follow-up material.
Templating the three Parameters sections through _fill_doc: already decided against in the previous round.
Routing numpy through _blocks as well: against the numpy-keeps-astropy policy.
Dask arrays with unknown chunk sizes fail in the trim slice: unrealistic input, and _nanfuncs fails the same way.
The invalid-block-size test also asserts astropy's live wording: shape chosen in the previous round.

🤖 Generated with Claude Code

https://claude.ai/code/session_017kEJ4Z2wTtZC4TzCQe8DQq

- _blocks: add _promote_for_division so complex input to block_average and block_replicate(conserve_sum=True) keeps its imaginary part instead of being cast to the default real dtype; new parity test against astropy.nddata for both functions on every backend. - _blocks: move block_reduce's trimming sentence under Notes; note that an explicit func=xp.sum bypasses the bool promotion; cite astropy #20360 in block_replicate's Notes. - core: _block_dispatch resolves the namespace itself via _namespace_for, so the three public wrappers are single dispatch calls; block_average's astropy callable is partial(nddata.block_reduce, func=np.mean); drop the stale reuse paragraph from _namespace_for's Notes. - tests: delete test_core_wrappers_rewrap_ccddata (covered by test_ccdproc.py on every backend); add a float64 canary for astropy #20360 to the float32 block_replicate test. - docs/CHANGES: cite astropy #20360 for the float32 upcast difference. - _blocks/core: document that an explicit xp must be the namespace of the data and is not checked. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017kEJ4Z2wTtZC4TzCQe8DQq

mwcraig · 2026-09-11T21:24:18Z

Pushed 72624c1 with everything agreed in the last round: _promote_for_division guard so complex input keeps its imaginary part in block_average and sum-conserving block_replicate (complex128 parity test added; _nanfuncs.py still identical to main, wider _nanfuncs complex handling tracked in #1016); _block_dispatch resolves the namespace itself and the three wrappers are single dispatch calls with np.mean in the astropy partial; stale reuse paragraph dropped; trimming sentence and the explicit-func=xp.sum note under block_reduce's Notes; "xp must match the data, not checked" on the native functions and the dispatcher; test_core_wrappers_rewrap_ccddata deleted; astropy/astropy#20360 cited in docs, CHANGES and the block_replicate Notes, with a float64 canary in the float32 replicate test.

Block tests: 87 passed on each of numpy, jax, dask and array-api-strict. Full suite locally: numpy 1048 passed / 35 skipped; strict 1004 passed / 36 skipped / 43 xfailed (all pre-existing).

— Written by Claude at @mwcraig's direction.

Rob2688 · 2026-09-15T07:42:45Z

Why is this still not merged.
I noticed this happening with multiple issues.

mwcraig · 2026-09-15T14:03:36Z

Why is this still not merged.
I noticed this happening with multiple issues.

That is an odd way to introduce yourself, but you are welcome to suggest changes to the PR and/or to review the code. I maintain the repo essentially by myself.

astropy.nddata's block functions start with numpy.asanyarray, so on a non-numpy array library they hand back a numpy array (dask, jax) or fail outright when the data are on a device numpy cannot reach (array-api-strict, cupy). ccdproc/_blocks.py does the same work using only array API operations -- reshape, permute_dims, repeat and slicing -- so the result stays in the caller's namespace and on the caller's device. block_size validation is pure Python and reproduces astropy's three checks, in astropy's order, with astropy's messages. Both functions are decorated with astropy.nddata.support_nddata, as astropy's own are, so a CCDData argument is unpacked and the "following attributes were set ... will be ignored" warning is emitted identically. The one deliberate divergence is dtype: block_replicate(conserve_sum=True) promotes integer and boolean input to the namespace's default real floating dtype before dividing, because array-api-strict rejects integer true division rather than promoting. numpy returns float64 there anyway. _nanfuncs._fill_doc gained a positional-only template argument so the new module can reuse the docstring templating with its own parameter block. Part of astropy#971. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN

ccdproc.core.block_reduce/block_average/block_replicate now resolve the array namespace first and keep calling astropy.nddata only when it is numpy; every other namespace gets ccdproc._blocks. The numpy branch is the code that was there before, so numpy results are unchanged, and the CCDData rebuild and the ignored-attribute warning are untouched. block_replicate gained the xp= argument the other two already had. With the escape gone, drop the three block_* lines from the array-escape baseline and the three array-api-strict xfail markers on the block tests. Those tests then failed one step later on xp.zeros(..., dtype=bool), which array-api-strict rejects; they now ask for xp.bool, as the rest of the file does. Part of astropy#971. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN

Part of astropy#971. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN

numpy's mean promotes an integer array on its own, and jax and dask follow it, but array-api-strict refuses a non-floating mean outright, so an integer image averaged fine on three backends and raised on the fourth. ccdproc._blocks.block_average now promotes integer and boolean input to the namespace's default real floating dtype before reducing with xp.mean, the same treatment block_replicate already had, and core.block_average dispatches to it. The numpy path is unchanged and still goes straight to astropy.nddata. block_average has no astropy counterpart, so it lives here as ccdproc's own thin wrapper; only block_reduce and block_replicate are candidates for the upstream lift. The docstring parameter template gained an empty-``extra`` form, since block_average has no function-specific parameter between block_size and xp. Part of astropy#971. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN

Codecov flagged two untested branches in astropy#1009: the early return in _block_namespace when the caller passes xp, and the int() failure branch in _block_size that turns a NaN or infinite block size into astropy's "must be integers" error. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN

…, dedup Production code: - Rename ``_block_namespace`` to ``_namespace_for``, a generic resolver that unwraps any ``NDData`` and falls back to the numpy namespace when no array library claims the input, so plain ``NDData`` and nested-list input reach ``astropy.nddata`` exactly as on ``main``. - Add ``_block_dispatch`` so the three ``block_*`` wrappers share the numpy/native branch and the ``CCDData`` re-wrap. - ``block_reduce`` with the default reduction casts boolean input to the namespace's default integral dtype, matching astropy's bool sum instead of failing on array-api-strict or narrowing on jax. - ``block_replicate`` divides the input once, in one statement, and replicates with reshape/broadcast_to/reshape; it keeps a floating input's dtype, now documented as the second difference from astropy. - ``_block_size`` integrality check reduced to ``float(s).is_integer()``. - Inline the three Parameters sections and drop the ``_fill_doc`` generalisation, so ``_nanfuncs.py`` is unchanged from ``main``. - Docstrings: short summaries, rationale under ``Notes``. Tests: NDData/list regression tests, bool and float32 parity cases, a ``CCDData`` re-wrap test, ``match=`` substrings instead of byte-equal error messages, and the two namespace/device tests merged. Docs and changelog shortened; the jax int32 default sum width is noted. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017kEJ4Z2wTtZC4TzCQe8DQq

- _blocks: add _promote_for_division so complex input to block_average and block_replicate(conserve_sum=True) keeps its imaginary part instead of being cast to the default real dtype; new parity test against astropy.nddata for both functions on every backend. - _blocks: move block_reduce's trimming sentence under Notes; note that an explicit func=xp.sum bypasses the bool promotion; cite astropy #20360 in block_replicate's Notes. - core: _block_dispatch resolves the namespace itself via _namespace_for, so the three public wrappers are single dispatch calls; block_average's astropy callable is partial(nddata.block_reduce, func=np.mean); drop the stale reuse paragraph from _namespace_for's Notes. - tests: delete test_core_wrappers_rewrap_ccddata (covered by test_ccdproc.py on every backend); add a float64 canary for astropy #20360 to the float32 block_replicate test. - docs/CHANGES: cite astropy #20360 for the float32 upcast difference. - _blocks/core: document that an explicit xp must be the namespace of the data and is not checked. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017kEJ4Z2wTtZC4TzCQe8DQq

astropy PR #20364 (in 7.2.3 and 8.0.2) fixed astropy/astropy#20360, so astropy.nddata.block_replicate(..., conserve_sum=True) now keeps float32 input as float32 on the devdeps job, tripping the dtype canary in test_block_replicate_float32_input_keeps_float32. Drop the canary: the test accepts either reference dtype and keeps comparing values after a cast to float32, so it passes on astropy with and without the fix. Word the documented difference in docs/array_api.rst and CHANGES.rst as applying to astropy before that fix. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

astropy PR #20393 (in 8.0.2) fixed astropy/astropy#20331: the compiled sigma_clip path now stops iterating a slice once an iteration rejects all of its remaining values and keeps that iteration's bounds, so the whole slice ends up masked. Before the fix it iterated on over the empty slice, read out of bounds and, when it did not crash, returned NaN bounds that left the slice's finite values unmasked. _sigma_clip_mask reproduced the old result; on the devdeps job its mask no longer matched astropy's in 33 mean/mad_std cases of test_sigma_clip_mask_matches_astropy. _sigma_clip_mask now keeps a slice's bounds once the next iteration's statistics come back NaN, which happens only when the slice was emptied, matching the fixed compiled path. The test reference reads NaN final bounds as "everything rejected", so it is the fixed behaviour on every astropy version, and checks astropy's own mask only where astropy bounds the slice, since unfixed versions leave an emptied slice unmasked; on a fixed astropy that check covers the whole array. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

mwcraig

Final pre-merge review of 5705207, focused on merge blockers and on reducing complexity and line count. The suite is green on numpy, dask, jax and array-api-strict in the local tox environments (astropy 8.0.1), so every blocker below is a path the tests do not exercise. Each was reproduced by hand against the branch.

Blockers (details inline)

core._namespace_for returns a raw module xp= unchanged, so block_reduce(dask_arr, 2, xp=dask.array) now raises AttributeError where main worked. (ccdproc/core.py)
_blocks.block_replicate returns a read-only broadcast view, aliasing the input with conserve_sum=False, whenever every axis has length 1 or block size 1. (ccdproc/_blocks.py)
The public wrappers are never tested with an explicit func or with conserve_sum=False, which is where 1 and 2 live. (ccdproc/tests/test_blocks.py)
CHANGES.rst cites [#971] instead of [#1009].
The sigma-clip change makes non-numpy backends differ from the numpy path on astropy < 8.0.2, and that is recorded only in the private _sigma_clip_mask docstring. The CHANGES entry for #1001 and the public Combiner.sigma_clipping Notes still claim unqualified parity. (ccdproc/combiner.py)

Line-count reductions (all behaviour-preserving, all run green on the four backends)

Where	Saves	Note
`test_blocks.py` / `test_ccdproc.py`	~175 of 454	eight oracle tests -> two parametrized ones; drop the astropy-message assertion; delete dead `HAS_BLOCK_X_FUNCS`
`_blocks.py`	~92 of 334	63% of the file is docstring in a private module; the data/block_size/xp entries are written out three times
`core.py`	~40	inline `_namespace_for` into its one caller; drop the extra numpy import
`combiner.py`	~12	the bound-freeze block reduces to `\| xp.isnan(lower)` in the final mask

About 85 of the test-file lines come from merging tests whose docstrings carry the rationale; the reasons would move to comments beside the parameter rows. That is the one item that may not fit the per-test-docstring rule.

Not blocking, noted inline: explicit func=xp.sum on bool input raises on strict while the default works; a dask array with unknown chunk sizes now fails with an opaque ValueError where main worked; jax without x64 keeps int32 block sums.

Separate, pre-existing, not this PR: on Python 3.13 _nanfuncs._fill_doc (from #1006) over-indents every continuation line of the shared parameter block, so nansum.__doc__ and friends render broken numpydoc. Python 3.13 dedents docstrings at compile time, so textwrap.indent(...).lstrip() is wrong there. Fix: keep _COMMON_PARAMS unindented and format inspect.cleandoc(func.__doc__). Worth landing before _fill_doc spreads into _blocks.py.

🤖 Generated with Claude Code

https://claude.ai/code/session_014HjXntgHjCLooJD9a4sC5p

Copilot

🟡 Changes recommended

Sigma clipping currently mistakes NaN-producing callable statistics for emptied slices.

Get a fresh assessment by requesting another Copilot review.

Review details

Files reviewed: 9/9 changed files
Comments generated: 1
Review effort level: Balanced

…ings CHANGES.rst (threads at CHANGES.rst:26, CHANGES.rst:32, combiner.py:236): - shorten this PR's block-functions entry to the agreed single sentence, dropping the dtype details that docs/array_api.rst already tells in prose, noting the new ``xp`` keyword on ``block_replicate``, and citing [astropy#1009] rather than the tracking issue [astropy#971]; - add a separate entry for the sigma-clip behaviour change, which follows astropy 8.0.2 in masking every value of a slice an iteration empties; - qualify the astropy#1001 parity claim, which is unqualified today, with a pointer to that new entry. ccdproc/tests/test_ccdproc.py (thread at test_ccdproc.py:1050): - delete the dead ``HAS_BLOCK_X_FUNCS`` try/except and the three ``skipif`` markers it guarded; the import is from ``ccdproc.core``, not astropy, so it can never fail on any supported astropy. The three names join the existing ``ccdproc.core`` import; - give ``test_block_reduce``, ``test_block_average`` and ``test_block_replicate`` docstrings saying what they uniquely pin: the ``CCDData`` round trip through the public wrappers (unit and meta copied, meta not aliased, mask/uncertainty/wcs dropped) and, for ``block_average``, that the single warning shows the nested ``support_nddata`` decorators do not warn twice. No test outcomes change: 98 passed on numpy, dask and jax, 89 passed and 9 xfailed on array-api-strict, before and after. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Review items from the astropy#1009 threads at combiner.py:236, combiner.py:324 (Copilot), combiner.py:327 and test_combiner.py:1803. The freeze block in _sigma_clip_mask stays as it is: the "| xp.isnan(lower)" simplification and Copilot's "detect emptiness from filtered" were both declined on the threads, with a counterexample, so this commit is docs and tests only. combiner.py: - The private Notes of _sigma_clip_mask now say explicitly that the emptied-slice rule is the string cenfunc/stdfunc case, the one most callers hit, and that a callable whose statistic goes NaN on a slice that is not empty freezes the bounds the same way, leaving only the earlier iterations' rejections masked (astropy >= 8.1's union). - The public Notes of Combiner.sigma_clipping record the divergence a caller can see: a slice an iteration empties ends up fully masked, as astropy does from 8.0.2, while on older astropy the numpy path leaves that slice's finite values unmasked. test_combiner.py: - _sigma_clip_reference no longer masks an emptied slice unconditionally. The term is gated on _ASTROPY_MASKS_EMPTIED_SLICE = minversion(astropy, "8.0.2") and on the namespace, so the reference produces what the code under test produces instead of passing only because the parametrized grid never empties a slice. A new via_sigma_clipping flag marks the three callers that compare against Combiner.sigma_clipping, the only route on which numpy data reach astropy's unfixed behaviour; the two callers that test _sigma_clip_mask directly always get the fixed semantics. A comment records that the flag, the numpy carve-out and "checked" all go once astropy 8.0.2 is the floor. - New test_sigma_clip_mask_callable_nan_statistic pins the callable scenario from the Copilot thread ([0, 0, 0, 0.05, 100], sigma=1, maxiters=5, stdfunc NaN below four remaining values): F F F T T on every backend, the case where the declined one-liner would mask all five. - New test_sigma_clipping_emptied_slice pins the string case through Combiner.sigma_clipping ([0., 1., 1.], mean/mad_std, maxiters=2): fully masked off numpy, and on numpy only once astropy masks it. Nothing covered that path before. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…d their tests core.py: - The three public wrappers get real numpydoc docstrings covering ``ccd``, ``block_size``, ``func``/``conserve_sum``, the previously undocumented ``xp`` and the astropy-vs-native split, and the ``block_reduce.__doc__ += nddata.block_reduce.__doc__`` splices go: astropy's text documents ``data`` rather than ``ccd``, has no ``xp``, and ``block_average`` never got one at all. - ``_namespace_for`` is folded into ``_block_dispatch``: one caller, a 28-line docstring around an 8-line body. An explicit ``xp`` is no longer returned verbatim but normalised with ``array_api_compat.array_namespace(xp.asarray(0))``, as ``Combiner`` does, so a plain module such as ``dask.array`` no longer reaches ``_blocks`` and raises ``AttributeError`` on ``isdtype``/``permute_dims``. - The ``import array_api_compat.numpy`` fallback is dropped; the fallback namespace is only ever fed to ``is_numpy_namespace``, for which plain ``np`` is ``True``. _blocks.py: - ``block_replicate`` copies its result when every axis has ``length == 1`` or ``size == 1``: no axis pair had to be merged, so the final ``reshape`` handed back a read-only view that, with ``conserve_sum=False``, aliased the caller's own data. - The boolean-to-integer promotion in ``block_reduce`` is gated on ``func is None or func is xp.sum``, so spelling out the documented default no longer raises on array-api-strict. - ``block_reduce`` and ``block_replicate`` reject a shape that is not fully known with a message naming ``compute_chunk_sizes()``, instead of dying inside Python arithmetic with "cannot convert float NaN to integer" for a dask array with unknown chunk sizes. - ``nblocks`` is hoisted in ``block_reduce`` and the same ``pairs`` tuple collapses the three zips in ``block_replicate``. - Docstrings are trimmed by about 75 lines: ``_block_size`` and ``_promote_for_division`` down to a short paragraph each, the module docstring loses the two paragraphs restating ``_block_dispatch``'s Notes and docs/array_api.rst, each function's Notes down to what a maintainer of this file needs, and ``block_average``'s extended summary moves under Notes. No behaviour, dtype, exception type or message changes. test_blocks.py: - One parametrized value-grid test over five native/astropy pairs and the ten shape/block-size cases replaces the three separate parity tests; the five dtype tests stay separate, each with its own reason. - New coverage for the two fixes above: a degenerate-block-size parity row on every backend plus aliasing and writability assertions (skipped on jax, whose arrays are immutable), and ``func=xp.sum`` on a boolean mask. - ``_CORE_WRAPPERS_AND_REFERENCES`` gains an explicit-``func`` row and a ``conserve_sum=False`` row, so the wrappers' argument forwarding is exercised, and ``test_core_wrappers_honour_an_explicit_xp`` gains a raw-module ``xp`` row. - The invalid-block-size test drops its assertion on astropy's live error text, absorbs the three non-finite rows so ``block_replicate`` gets them too, and is renamed accordingly; redundant ``isdtype`` assertions and the default-dtype lookup go. docs/array_api.rst: one sentence on the fully-known-shape precondition. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

mwcraig · 2026-09-17T16:18:03Z

Pushed the agreed fixes from the 2026-09-15 review as three commits on top of 5705207 (nothing rebased):

6223f07 CHANGES.rst (short block entry with [#1009], separate sigma-clip entry, qualifier on the Array-API fallback for Combiner.sigma_clipping when the namespace is not numpy #1001 entry) and test_ccdproc.py (dead HAS_BLOCK_X_FUNCS guard and skipif markers gone, docstrings on the three CCDData block tests).
b0acedc sigma clipping: the freeze block is unchanged; private and public Notes now record the emptied-slice behaviour and the callable case; _sigma_clip_reference is gated on minversion(astropy, "8.0.2") for the Combiner-level callers; two new tests, one for the callable-returns-NaN-on-a-nonempty-slice scenario (F F F T T on every backend) and one for the string-statistics emptied slice through Combiner.sigma_clipping.
51ceb52 block functions: raw-module xp= normalised in _block_dispatch (which absorbed _namespace_for), block_replicate copies its result in the degenerate case, func is xp.sum takes the bool promotion, nblocks/pairs hoists, a clear ValueError for dask arrays with unknown chunk sizes, real docstrings on the three public wrappers, _blocks.py docstrings trimmed (334 → 272 lines), and the test-file changes agreed in the threads (value grid, explicit-func/conserve_sum=False/raw-module rows, degenerate-replicate and writability tests, _INVALID_BLOCK_SIZES fold, no more asserts on astropy's error text).

Each thread has a reply pointing at the file and line. Full suite locally on astropy 8.0.1: numpy 1075 passed, dask 1071, jax 1071, array-api-strict 1031 passed / 43 xfailed with no escapes logged. Threads left for you to resolve.

— Written by Claude at @mwcraig's direction.

mwcraig

Final pass over 51ceb52 (/code-review 1009 high, subagent-assisted). Scope was blockers and large simplifications; small stuff is deliberately left out.

Recommend fixing before merge

_blocks.py:268 — block_replicate returns a read-only broadcast view for any zero-length axis.
_blocks.py:137 — block_reduce promotes int/bool input only when func is xp.sum; func=xp.mean (the advertised block_average equivalent) raises on array-api-strict where numpy returns float.
test_combiner.py:2182 — the numpy-branch assertion on astropy < 8.0.2 pins the result of an out-of-bounds read (astropy#20331), and oldestdeps runs astropy 6.1.

Optional simplifications
4. _blocks.py:268 — the defensive copy also runs on the conserve_sum=True path where data is already private.
5. core.py:1804 — third copy of the array_namespace(module.asarray(0)) normalisation (also combiner.py:436, core.py:1405).
6. _blocks.py:200 — block_average pays support_nddata twice.

Wording
7. combiner.py:239/:755 and CHANGES.rst:27 say an emptied slice ends up fully masked; the frozen-bounds final mask does not guarantee that.
8. core.py:1797 — the bare except TypeError also swallows a failure from a foreign array's own __array_namespace__ (speculative, no verified trigger).

Details inline.

Five follow-ups from the review of the block-function commit: * ``_blocks.block_replicate`` now also copies when an axis is empty: the final ``reshape`` hands back a read-only view for a zero-length axis whatever the block size, where astropy's ``numpy.repeat`` always gives a fresh writable array. Pinned by two new ``_DEGENERATE_REPLICATE_CASES``. * ``_blocks.block_reduce`` promotes integer and boolean input for an explicit ``func=xp.mean``, as it already did for ``xp.sum``, so the one call ``block_average`` exists to serve no longer raises on array-api-strict while working on numpy, dask and jax. Documented, along with the fact that both promotions are keyed on the exact function objects, so a ``partial`` or ``lambda`` opts out. * A ``core._namespace_from_module`` helper carries the module-normalisation rationale once, replacing the three hand-written copies in ``_block_dispatch``, ``Combiner.__init__`` and ``combine``. * ``core._block_dispatch`` re-raises a ``TypeError`` coming from an array's own ``__array_namespace__`` instead of falling back to numpy, where it resurfaced as a ``block_size`` ``ValueError``. The ``hasattr`` gate runs only after the lookup has failed, because dask arrays have no dunder and a gate placed first would route them back through astropy. * ``block_reduce`` and ``block_average`` share an undecorated ``_block_reduce``, so the second ``support_nddata`` hop, and the comment explaining why it was harmless, both go away. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M8AJaygzvm8VuDf7Sq5HSE

The rebase onto main (after astropy#1009) left the baseline correct in content but not in layout: astropy#1009 removed the three block_* entries and this branch removes the ``subtract_overscan`` one, and that entry was the last ``numpy.asanyarray`` in the file, so the coercion column is now narrower. Regenerated the documented way -- a full dask run with CCDPROC_WRITE_ESCAPE_BASELINE=1 -- rather than by hand, so the checked-in file is byte-for-byte what a refresh produces. No entry was added or dropped by the regeneration. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M8AJaygzvm8VuDf7Sq5HSE

The rebase resolved this file by content -- the three scipy.ndimage entries this branch removes are gone, and the ones astropy#1009 removed stay gone -- but the column widths were still those of the pre-rebase file, sized for the longest function name at the time. Regenerating it the documented way (CCDPROC_WRITE_ESCAPE_BASELINE=1 with the dask backend over a full suite run) re-pads the columns to the four entries that are actually left, so the next refresh produces no spurious diff. No entry is added, removed or retagged by this commit. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M8AJaygzvm8VuDf7Sq5HSE

astropy#1009's review added the array-like guard in ``core._block_dispatch``: resolve the namespace, and if no array library claims the input, fall back to numpy rather than letting ``array_namespace`` raise -- but re-raise when the input does advertise ``__array_namespace__``, so a genuine namespace failure is not hidden behind a numpy coercion. ``_median_filter_array``, added on this branch before astropy#1009 merged, resolves its namespace with a bare ``array_namespace`` and so inherits neither half. That regressed ``ccdproc.median_filter``, which documents itself as a passthrough for ``scipy.ndimage.median_filter`` and took any array-like on every previous release: a nested list now raised ``TypeError: list is not a supported array type`` instead of being filtered. Adopting the same guard sends lists and tuples back to ndimage, exactly as before, and leaves every array input untouched. Tests: a nested list and a tuple of lists go to ndimage on every backend, and an object whose ``__array_namespace__`` lookup fails still raises rather than being quietly coerced (ndimage does not raise for one, so the guard is what the test pins). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M8AJaygzvm8VuDf7Sq5HSE

mwcraig mentioned this pull request Sep 7, 2026

Array-API-native k×k window filters for cosmicray_median, median_filter, background_deviation_filter and ccdmask #1010

Open

mwcraig mentioned this pull request Sep 7, 2026

Make the host round trip explicit in wcs_project, subtract_overscan and cosmicray_lacosmic #1011

Open

mwcraig force-pushed the blocks-native branch from fdaad17 to 34721db Compare September 10, 2026 14:07

mwcraig requested a balanced review from Copilot September 10, 2026 14:08

Copilot started reviewing on behalf of mwcraig September 10, 2026 14:09 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Comment thread ccdproc/core.py Outdated

mwcraig commented Sep 10, 2026

View reviewed changes

This was referenced Sep 10, 2026

docs: array_api.rst limitations section is turning into a changelog; restructure after the array-API PRs land #1014

Open

Consider promoting the _to_xp test helper to a shared fixture #1015

Open

mwcraig requested a balanced review from Copilot September 10, 2026 18:05

Copilot started reviewing on behalf of mwcraig September 10, 2026 18:05 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Comment thread ccdproc/_blocks.py Outdated

mwcraig commented Sep 10, 2026

View reviewed changes

mwcraig mentioned this pull request Sep 11, 2026

Complex input to the _nanfuncs reductions loses its imaginary part on non-NumPy backends #1016

Open

mwcraig and others added 6 commits September 15, 2026 09:04

Document the native block functions in the docs and changelog

757d4e9

Part of astropy#971. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F6f9L1GyMHrKnvLrrgWbWN

mwcraig force-pushed the blocks-native branch from 72624c1 to 72d5c24 Compare September 15, 2026 14:04

mwcraig and others added 2 commits September 15, 2026 09:30

mwcraig commented Sep 15, 2026

View reviewed changes

Comment thread ccdproc/core.py

Comment thread CHANGES.rst

mwcraig commented Sep 15, 2026

View reviewed changes

mwcraig requested a balanced review from Copilot September 17, 2026 14:46

Copilot started reviewing on behalf of mwcraig September 17, 2026 14:47 View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

Comment thread ccdproc/combiner.py

mwcraig and others added 3 commits September 17, 2026 11:14

mwcraig mentioned this pull request Sep 17, 2026

Cycle 5: Array API in ccdproc and NDData and work on astrowidgets (Craig) astropy/astropy-project#515

Open

mwcraig commented Sep 17, 2026

View reviewed changes

mwcraig merged commit 0c21068 into astropy:main Sep 17, 2026
30 checks passed

mwcraig deleted the blocks-native branch September 17, 2026 22:48

Uh oh!

Conversation

mwcraig commented Sep 7, 2026

Dispatch policy

Behaviour differences

Verified backends

Things worth a reviewer's eye

Uh oh!

codecov Bot commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mwcraig left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mwcraig commented Sep 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

mwcraig left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mwcraig commented Sep 11, 2026

Uh oh!

Rob2688 commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mwcraig commented Sep 15, 2026

Uh oh!

Uh oh!

Uh oh!

mwcraig left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Sep 7, 2026 •

edited

Loading

Rob2688 commented Sep 15, 2026 •

edited

Loading