Make combiner internals array-API compatible - #988
Merged
Conversation
Fix the strict-backend bugs in the Combiner internals and the memory sizing used by combine(): - Replace the numpy-only mask.any() call in _get_nan_substituted_data with xp.any(), and count masked entries with xp.count_nonzero() instead of the .sum() array method (sums of booleans are not allowed under the array API in any case). - Replace len() on stacked data arrays with .shape[0] -- the array API does not require arrays to implement __len__ -- and cast the resulting integer counts to float64 before xp.sqrt() and the uncertainty scaling, matching the int-to-float promotion numpy does implicitly and strict namespaces refuse. - Pass the weights reshape target as a tuple; strict namespaces reject a list. - Use xp.bool instead of the builtin bool for mask dtypes in the _CCDDataWrapperForArrayAPI mask setters and in tests that build masks in the testing namespace; strict namespaces only accept their own dtype objects. - Size images for the mem_limit calculation in combine() without .nbytes, which is not part of the array API standard: a new _array_size_in_bytes() helper computes the size from the element count and the dtype bit width reported by xp.finfo()/xp.iinfo(). The backend_xfail guarding test_combine_scale_callable_* is removed now that combine() no longer fails on .nbytes, and a direct unit test covers the new sizing. With CCDPROC_ARRAY_LIBRARY=array-api-strict the test suite goes from 99 to 85 failures (348 to 364 passes); the remaining failures are known separate issues (device handling, xp.median, clip_extrema fancy indexing, sigma clipping via astropy.stats) or numpy-isms in test bodies. 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 Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #988 +/- ##
==========================================
+ Coverage 97.29% 97.42% +0.13%
==========================================
Files 9 9
Lines 1738 1749 +11
==========================================
+ Hits 1691 1704 +13
+ Misses 47 45 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Codecov flagged the complex path as the one uncovered line in the patch; no combiner test feeds the size helper a complex array. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RQMJZUaaxfqGDk41GLSaFK
mwcraig
commented
Aug 24, 2026
| the array API standard and use them in ``Combiner.average_combine`` and | ||
| ``Combiner.sum_combine`` when the selected array namespace has no | ||
| ``nansum``/``nanmean``/``nanstd``, instead of raising ``RuntimeError``. [#986] | ||
| - Make the ``Combiner`` combination internals array-API compatible: masked |
Member
Author
There was a problem hiding this comment.
Agreed — the mechanism list belongs in the PR, not the changelog. Proposed replacement for the whole entry:
- Make the ``Combiner`` combination internals array-API compatible and size
images for ``mem_limit`` from the element count and dtype width instead of
the non-standard ``nbytes``. [#988]Happy to push that.
— Written by Claude at @mwcraig's direction.
Member
Author
Contributor
There was a problem hiding this comment.
Pull request overview
Makes Combiner internals compatible with strict Array API backends.
Changes:
- Replaces NumPy-specific array methods and dtype usage.
- Adds portable array memory-size calculation.
- Expands tests and updates the changelog.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
CHANGES.rst |
Documents the compatibility fixes. |
ccdproc/combiner.py |
Updates reductions, sizing, reshaping, and uncertainty scaling. |
ccdproc/_ccddata_wrapper_for_array_api.py |
Uses namespace-native boolean dtypes. |
ccdproc/tests/test_combiner.py |
Updates backend-compatible tests and adds memory-size coverage. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The mechanism list belongs in the PR description, not the changelog. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RQMJZUaaxfqGDk41GLSaFK
55 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #971. This addresses three unnumbered checklist items from the array-API tracker, all in the combiner internals, plus the immediately adjacent numpy-isms those fixes exposed in the same code paths.
The fixes
Combiner._get_nan_substituted_datausedmask.any()— a numpy-only array method. Nowxp.any(...)in the combiner's namespace. Fixing this exposed the next layer in the same functions, which is also fixed here:.sum(axis=0)method calls (nowxp.count_nonzero, since the array API also disallows summing booleans),len(<array>)(now.shape[0];__len__is not required by the standard), a list passed toxp.reshape(now a tuple), and integer counts fed toxp.sqrt/uncertainty scaling (now cast tofloat64, matching the promotion numpy does implicitly).boolpassed as a dtype in the_CCDDataWrapperForArrayAPImask setters and in test bodies that build masks in the testing namespace — strict namespaces only accept their own dtype objects. Nowxp.bool.combine()memory sizing used.nbytes, which is not in the array API standard. A new_array_size_in_bytes()helper computes size fromarray_api_compat.size(arr)and the dtype bit width fromxp.finfo/xp.iinfo(booleans counted as 1 byte, complex as two components). The stalebackend_xfailontest_combine_scale_callable_returning_backend_scalaris removed and a direct unit test of_calculate_size_of_imageis added.Strict suite: 99 failed / 348 passed → 85 failed / 364 passed
(
CCDPROC_ARRAY_LIBRARY=array-api-strict, on top of #987.)Causes eliminated: the 28
.any()failures, the 9 builtin-booldtype failures, the 2.nbytesfailures, and the intermediate layers behind them (.summethod,len(), list reshape, int→float promotion). One previously xfailed test now passes and its marker is removed; threetest_cosmicrayparametrizations now xpass as a side effect of the wrapper mask-setter fix (their marker still guards other parametrizations, so it stays).Remaining failures are known separate issues, left for follow-up: device handling (#946, now the largest bucket at 31 since more tests reach the device checks),
xp.medianincore.py(4), fancy-index__setitem__inclip_extrema(4), sigma clipping viaastropy.stats(#929),combine()file-input handling, CCDData arithmetic converting arrays insideastropy.nddata(the "Could not convert Array" bucket, hit by the scaling tests), and numpy-isms in test bodies (.mean()method calls, mixed int/float test arithmetic, dtype comparisons).test_combiner.pypasses fully on numpy (89 passed) and dask (93 passed); the full numpy suite is green (501 passed).🤖 Generated with Claude Code
https://claude.ai/code/session_01RQMJZUaaxfqGDk41GLSaFK