Build Combiner data and mask arrays with xp.stack

[mwcraig]

array-api-strict rejects a nested list of arrays passed to xp.asarray, so every Combiner construction failed under the strict backend (50 of the 75 test_combiner.py failures). This PR:

• builds _data_arr with xp.stack([...]) + xp.astype(..., dtype);
• builds _data_arr_mask by coercing each mask with xp.asarray(mask, dtype=xp.bool, device=<data device>) (astropy keeps CCDData.mask as numpy even when the data is not) and a dtype=xp.bool, device=... placeholder for missing masks, then xp.stack;
• replaces the remaining dtype=bool with dtype=xp.bool;
• puts scaling arrays on the device of the data, reshapes them for broadcasting with a single xp.reshape, and indexes individual images with a trailing ellipsis (strict rejects both repeated [:, newaxis] on an N-D array and flat arr[i] indexing);
• adds a regression test that builds a Combiner from data on the testing device with a native-namespace mask, no mask, and a numpy mask, and checks the namespace, device, dtype, and values of .data, .mask, and .scaling.

Test matrix (local, ccdp-dev-py312):

backend	result
numpy	381 passed, 5 skipped
jax	367 passed, 10 skipped, 2 xfailed, 7 xpassed (all pre-existing XPASSes on main)
dask	371 passed, 15 skipped
dask + CCDPROC_ENFORCE_ESCAPE_BASELINE=1	371 passed, 15 skipped; no escapes outside the baseline
array-api-strict, test_combiner.py	before: 75 failed, 12 passed; after: 68 failed, 19 passed, 1 skipped

The remaining strict failures in test_combiner.py are unrelated to this issue: 35 No NaN-aware ... function available (bottleneck fallbacks), 6 dtype=bool in the tests themselves, 6 IrreducibleUnit passed to xp.asarray, 5 numpy.ndarray has no attribute _array, 3 flat-indexing IndexErrors in test code, 3 Fancy indexing __setitem__, and a handful of others.

Fixes #965

🤖 Generated with Claude Code

https://claude.ai/code/session_01DTN9DnPnLKK2u7knnMJ2gA

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.90%. Comparing base (89e90f9) to head (1570989).
⚠️ Report is 16 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #976      +/-   ##
==========================================
+ Coverage   95.83%   95.90%   +0.07%     
==========================================
  Files           8        8              
  Lines        1631     1637       +6     
==========================================
+ Hits         1563     1570       +7     
+ Misses         68       67       -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.

[Copilot]

Pull request overview

This PR updates Combiner construction and related scaling/mask handling to be compatible with strict Array API backends (notably array-api-strict) by avoiding nested-array coercions and keeping derived arrays on the input data’s namespace/device.

Changes:

• Build Combiner’s internal data/mask stacks using xp.stack (and per-mask coercion) instead of xp.asarray on nested sequences.
• Ensure scaling arrays are created/reshaped in a strict-backend-friendly way (device-aware, broadcastable shape, strict indexing).
• Add a regression test covering mixed mask sources (native namespace mask, no mask, numpy mask) on the testing device.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
CHANGES.rst	Documents the Combiner stacking/device behavior fix.
ccdproc/tests/test_combiner.py	Adds a regression test for stacking + device/namespace preservation and scaling device placement.
ccdproc/combiner.py	Switches internal array construction to xp.stack, fixes bool dtype usage, and makes scaling device-aware and strict-indexing compatible.
ccdproc/init.py	Adds a blank line after the module docstring (formatting-only).

---

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

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

ccdproc/combiner.py:286

• In the callable-scaling branch, self._scaling becomes a Python list of per-image results and is then passed to xp.asarray(...). If the scaling function returns backend scalars/0-D arrays (common for array-API code, e.g. xp.mean(image)), array-api-strict will reject this as a nested sequence of Arrays (same failure mode as #965). Building the 1-D scaling array via xp.stack over xp.asarray-coerced scalars avoids nested-array rejection and still keeps the result on the chosen device.

This issue also appears on line 1126 of the same file.

            if callable(value):
                self._scaling = [value(self._data_arr[i, ...]) for i in range(n_images)]
                self._scaling = xp.asarray(self._scaling, device=device)

ccdproc/combiner.py:1128

• When scale is callable, scalevalues is built from scale(imgccd.data) and then converted with xp.asarray(scalevalues, ...). If scale(...) returns backend scalars/0-D arrays, this again becomes a nested sequence of Arrays and will be rejected by array-api-strict. Stacking per-element xp.asarray results avoids nested-array errors while preserving the intended device placement.

            to_set_in_combiner["scaling"] = xp.asarray(
                scalevalues, device=array_api_compat.device(ccd.data)
            )

[mwcraig]

Re Copilot's two suppressed comments (Combiner.scaling setter and the callable-scale branch of combine()): confirmed and fixed in b67e5fe. Under array-api-strict, a scaling callable returning a 0-d backend array (e.g. xp.mean(img)) made xp.asarray([...]) raise the same "Nested Arrays are not allowed. Use stack instead." error as #965. Both sites now do xp.stack([xp.asarray(v, device=device) for v in values]), which handles both 0-d arrays and plain Python floats (xp.stack alone would break the float case the existing tests use). Added a strict-exercised check to test_combiner_stacks_arrays_on_input_device and a new test_combine_scale_callable_returning_backend_scalar (xfail on strict only because combine() still calls .nbytes, which is pre-existing and out of scope). Verified under numpy, dask, strict, and the dask escape ratchet.

— Written by Claude at @mwcraig's direction.

[mwcraig]

Critical review of the diff (numpy-regression, array-API-semantics, and test-coverage passes, every claim re-run locally).

Verdict: sound. numpy/dask behaviour is bit-identical to main across ~70 combinations (int/float/mixed dtypes, numpy/int/float/MaskedArray masks, missing masks, scaling as list/tuple/ndarray/0-d/callable, 1-D and 4-D images, combine() with mem_limit chunking × scale × method). Peak memory is lower than main (10×1000² float64→float64: 170 MB → 100 MB, since asarray(list) built an intermediate). The device-pinning commit holds on strict (device1) and jax for every mixed-device / mixed-namespace ordering tried. jax suite has the same 6 pre-existing XPASSes as main; all 62 remaining strict failures in test_combiner.py are outside this diff (30 bottleneck-missing, 6 dtype=bool in the test wrapper, .nbytes, fancy-index __setitem__, …); no nested-asarray remains anywhere.

Inline: two small strict-only bugs the PR leaves reachable in the scaling setter (int scaling; backend array as scaling), one policy question about raw modules as xp, one CHANGES note, and two honesty notes on what the new tests actually guard.

— Written by Claude at @mwcraig's direction.

[mwcraig]

Integer scaling breaks on array-api-strict. The setter never casts to the data dtype, so scaling=[1, 2], or a callable returning a Python int, yields an int64 array and self._data_arr * self.scaling (line 491) raises

TypeError: array_api_strict.float64 and array_api_strict.int64 cannot be type promoted together

numpy/dask/jax promote silently so nobody sees it today, but it is exactly the class of bug the strict job exists for, and this PR is what makes the scaling path reachable there. One token fixes it here and on line 308:

xp.asarray(value(self._data_arr[i, ...]), dtype=self._data_arr.dtype, device=device)

That also covers a callable that returns an int for some images and a float for others — xp.stack of mixed 0-d dtypes fails on strict too. Suggest adding c.scaling = [1, 2] and c.scaling = lambda arr: 1 to test_combiner_stacks_arrays_on_input_device.

— Written by Claude at @mwcraig's direction.

[mwcraig]

Fixed in 1570989: both the callable and array branches now pass dtype=self._data_arr.dtype. Added c.scaling = [1, 2, 3] and c.scaling = lambda arr: arr.shape[0] to test_combiner_stacks_arrays_on_input_device with dtype assertions; passes on array-api-strict.

— Written by Claude at @mwcraig's direction.

[mwcraig]

A backend array as scaling gives a misleading error on strict. Standard arrays need not implement __len__ (array-api-strict's doesn't), so

c.scaling = xp.asarray([1.0, 0.5], device=device)

is caught by this try/except and re-raised as "scaling must be a function or an array the same length as the number of images" even though the length is right. Pre-existing, but newly reachable. value.shape[0] (falling back to len() for lists/tuples) is standard-compliant.

— Written by Claude at @mwcraig's direction.

[mwcraig]

Fixed in 1570989: the length check now uses value.shape[0] when there is a shape, falling back to len(). Test adds a strict backend array as scaling, plus the wrong-length ValueError and scalar TypeError cases.

— Written by Claude at @mwcraig's direction.

[mwcraig]

Behaviour change for a raw module passed as xp. Combiner(ccds, xp=dask.array) constructed fine on main; now da.asarray(..., device=...) raises TypeError: from_array() got an unexpected keyword argument 'device'. Same for raw numpy 1.26 (xp.bool, device=).

Mitigating: only the bare modules are affected (the array_api_compat wrappers work, combine() already normalises array_package), and on main the same raw-dask object died one call later in clip_extrema (module 'dask.array' has no attribute 'argsort'), while core.py on main already passes device= in five places. So this was never really supported — but it's worth deciding as policy. Cheapest hardening, mirroring what combine() does:

xp = array_api_compat.array_namespace(xp.asarray(0)) if xp is not None else ...

or just say in the docstring that xp must be an array-API-compatible namespace.

— Written by Claude at @mwcraig's direction.

[mwcraig]

Do the hardening

[mwcraig]

Done in 1570989: an explicit xp is normalised with array_api_compat.array_namespace(xp.asarray(0)), the docstring says a plain module is accepted, and test_combiner_accepts_raw_module_as_namespace passes xp=np and checks _xp is the compat namespace.

— Written by Claude at @mwcraig's direction.

[mwcraig]

Minor, worth a line in CHANGES: this replaces main's "append one axis per data dim" loop, so a (3, 1) scaling array or a callable returning a 1-element array now yields (3, 1, 1) instead of (3, 1, 1, 1), and average_combine() on (3, 3) images returns (3, 3) instead of (3, 3, 3). That is a fix, but it is user-visible.

— Written by Claude at @mwcraig's direction.

[mwcraig]

Added to the CHANGES entry in 1570989.

— Written by Claude at @mwcraig's direction.

[mwcraig]

Honesty note on "regression test": this currently cannot regress on any backend. On numpy/dask the pre-fix xp.asarray(scalevalues) already passed; on strict, combine() dies earlier at ccd.data.nbytes (combiner.py:829), which is why it is xfailed. The xfail reason is accurate, but until .nbytes is dealt with, the combine() half of the scaling fix (lines 1134–1139) is exercised by nothing. Suggest rewording the comment to say so.

— Written by Claude at @mwcraig's direction.

[mwcraig]

Comment reworded in 1570989 to say the test cannot yet fail because of the scaling path on any backend, and why.

— Written by Claude at @mwcraig's direction.

[mwcraig]

This test guards the device logic only on array-api-strict. Traced per backend: on numpy, array_namespace(np_arr) is xp is true so the "different namespace" branch never runs; on dask, an unconditional da.asarray(np_arr, device='cpu') is accepted anyway so the old bug is undetectable; on strict it does catch it (ValueError: Unsupported device 'cpu') — but on main-strict it fails at the unrelated dtype=bool mask line first. Not wrong, just weaker than the comment suggests, and the strict job is currently continue-on-error. Worth either noting in the comment or adding an assertion on the device that fails on numpy too (e.g. pass xp=array_api_strict explicitly in this test).

— Written by Claude at @mwcraig's direction.

[mwcraig]

update the comment

[mwcraig]

Comment updated in 1570989 to state that only the array-api-strict job can fail this test, with the per-backend reasons.

— Written by Claude at @mwcraig's direction.