From dcf3f8d50f50be59726dd636990c9cd7cd2b1582 Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Sun, 30 Aug 2026 18:10:49 -0500 Subject: [PATCH 1/4] Make the array-api-strict suite green: fix flat_correct mask check flat_correct checked for masked flat pixels with the mask.any() method, which arrays of spec-only array-API namespaces (array-api-strict) do not have; use xp.any instead. Verified on numpy, dask, jax and array-api-strict. The two remaining strict failures are astropy-upstream and get accurate backend_xfail marks in line with the rest of the suite: - test_unit_mismatch_behaves_as_expected: astropy's arithmetic mixin builds the Quantity with `data << unit`, and array-api-strict rejects the astropy unit operand (see #936). - test_rebin_ccddata[True-True]: astropy's CCDData.mask setter coerces the mask with np.asarray, which fails for an array on the non-default device. The stale xfail on test_flat_correct_masked_flat_with_immutable_array (its reason described an older failure mode) is removed; the test passes on every backend with the fix. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01LVxnTLrWKStBdPwcmDNhxA --- CHANGES.rst | 4 ++++ ccdproc/core.py | 2 +- ccdproc/tests/test_ccdproc.py | 11 ++++++----- ccdproc/tests/test_rebin.py | 6 ++++++ 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 7317bd3b..011af9ce 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -37,6 +37,10 @@ Other Changes and Additions Bug Fixes ^^^^^^^^^ +- ``flat_correct`` now checks whether the flat has any masked pixels with + ``xp.any`` instead of the ``mask.any()`` method, which arrays of + spec-only array-API namespaces such as ``array-api-strict`` do not + have. [#1005] - Exclude masked and clipped pixels, and their weights, when computing weighted average combinations. [#952] - ``cosmicray_median`` now honors the mask of a ``CCDData`` or masked-array diff --git a/ccdproc/core.py b/ccdproc/core.py index 846d86ad..ad5e7e85 100644 --- a/ccdproc/core.py +++ b/ccdproc/core.py @@ -1266,7 +1266,7 @@ def flat_correct(ccd, flat, min_value=None, norm_value=None, xp=None): # Set masked values to unity; the array element remains masked, but the data # value is set to unity to avoid runtime divide-by-zero errors that are due # to a masked value being set to 0. - if _flat_normed.mask is not None and _flat_normed.mask.any(): + if _flat_normed.mask is not None and xp.any(_flat_normed.mask): _flat_normed.data = xpx.at(_flat_normed.data)[_flat_normed.mask].set(1.0) # divide through the flat diff --git a/ccdproc/tests/test_ccdproc.py b/ccdproc/tests/test_ccdproc.py index 27ef1ab5..37f20f49 100644 --- a/ccdproc/tests/test_ccdproc.py +++ b/ccdproc/tests/test_ccdproc.py @@ -754,6 +754,12 @@ def test_subtract_dark_fails(): subtract_dark(ccd_data, small_master) +@pytest.mark.backend_xfail( + "array-api-strict", + reason="astropy's arithmetic mixin builds the Quantity with " + "``data << unit``, and array-api-strict rejects the astropy unit " + "operand (astropy is not array-API aware, see #936)", +) def test_unit_mismatch_behaves_as_expected(): ccd_data = ccd_data_func() """ @@ -818,11 +824,6 @@ def test_flat_correct(): assert flat_data.header == ccd_data.header -@pytest.mark.backend_xfail( - "array-api-strict", - reason="the CCDData array-API wrapper passes the Python bool type to " - "array-api-strict when copying a mask", -) def test_flat_correct_masked_flat_with_immutable_array(): ccd_values = [[8.0, 8.0], [8.0, 8.0]] flat_values = [[2.0, 0.0], [4.0, 8.0]] diff --git a/ccdproc/tests/test_rebin.py b/ccdproc/tests/test_rebin.py index 66aa41bb..65d8f5bd 100644 --- a/ccdproc/tests/test_rebin.py +++ b/ccdproc/tests/test_rebin.py @@ -64,6 +64,12 @@ def test_rebin_smaller(): # test rebinning with ccddata object +@pytest.mark.backend_xfail( + "array-api-strict", + reason="astropy's CCDData.mask setter coerces the mask with " + "np.asarray, which fails for an array-api-strict array on the " + "non-default device", +) @pytest.mark.parametrize("mask_data, uncertainty", [(False, False), (True, True)]) def test_rebin_ccddata(mask_data, uncertainty): ccd_data = ccd_data_func(data_size=10) From 2528a315dc646f7f829fc390e41ae70ed9939912 Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Sun, 30 Aug 2026 18:11:21 -0500 Subject: [PATCH 2/4] Promote the strict array-API CI job into the regular test matrix The array-api-strict suite is green (839 passed, 46 xfailed locally), so retire the expected-failures carve-out: move py313-strict into the ci-tests matrix, where a test failure fails CI, and remove the continue-on-error step, the outcome artifact plumbing, and the strict_status.yml reporting workflow that existed only to surface the carve-out's real outcome on PRs. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01LVxnTLrWKStBdPwcmDNhxA --- .github/workflows/ci_tests.yml | 82 ++-------------- .github/workflows/strict_status.yml | 147 ---------------------------- CHANGES.rst | 5 + 3 files changed, 13 insertions(+), 221 deletions(-) delete mode 100644 .github/workflows/strict_status.yml diff --git a/.github/workflows/ci_tests.yml b/.github/workflows/ci_tests.yml index 57f4346f..b3c36cf0 100644 --- a/.github/workflows/ci_tests.yml +++ b/.github/workflows/ci_tests.yml @@ -79,6 +79,14 @@ jobs: tox_env: 'py312-alldeps-dask-enforce-cov' codecov_flags: 'dask' + # array-api-strict on a non-default device, a CPU-only proxy for + # CuPy device behavior. Promoted from the expected-failures job: + # the suite is green on this backend, so failures now fail CI. + - name: 'ubuntu-py313-strict' + os: ubuntu-latest + python: '3.13' + tox_env: 'py313-strict' + - name: 'windows-py312' os: windows-latest python: '3.12' @@ -131,77 +139,3 @@ jobs: uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 with: flags: ${{ matrix.codecov_flags }} - - # The strict array-API job is in its own matrix so that its failures -- - # expected until the remaining array-API bugs are fixed -- stay visible - # without cancelling the main test matrix above. The test step uses - # continue-on-error so the job (and the PR checks rollup) stays green; - # the real outcome is reported as a warning annotation, in the step - # summary, and via a per-matrix-entry strict-job-outcome-* artifact that - # strict_status.yml turns into a check run on the PR. - ci-tests-expected-failures: - name: ${{ matrix.name }} - runs-on: ${{ matrix.os }} - if: "!(contains(github.event.head_commit.message, '[skip ci]') || contains(github.event.head_commit.message, '[ci skip]'))" - strategy: - matrix: - include: - - name: 'ubuntu-py313-strict' - os: ubuntu-latest - python: '3.13' - tox_env: 'py313-strict' - - steps: - - name: Check out repository - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - fetch-depth: 0 - - name: Set up Python ${{ matrix.python }} - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 - with: - python-version: ${{ matrix.python }} - - name: Install base dependencies - run: | - python -m pip install --upgrade pip - python -m pip install tox wheel - - name: Print Python env - run: | - python --version - python -m pip list - - name: Run tests - id: tests - continue-on-error: true - run: | - tox -e ${{ matrix.tox_env }} -- ${{ matrix.toxposargs }} - # always() so an outcome file is recorded even when an earlier step - # failed; a skipped/cancelled tests step is recorded as "error" (an - # infrastructure failure) rather than being conflated with an expected - # test failure. The JSON schema here is the contract with - # strict_status.yml: {"name": "", "state": "success" | - # "failure" | "error"}. - - name: Record test outcome - if: always() - run: | - case "${{ steps.tests.outcome }}" in - success) state='success' ;; - failure) state='failure' ;; - *) state='error' ;; - esac - printf '{"name": "%s", "state": "%s"}\n' '${{ matrix.name }}' "$state" > strict-outcome.json - if [ "$state" = "failure" ]; then - echo "::warning::${{ matrix.tox_env }} tests failed (expected until the remaining array-API bugs are fixed)" - echo ":warning: **${{ matrix.name }}**: tests **failed** (expected until the remaining array-API bugs are fixed)" >> "$GITHUB_STEP_SUMMARY" - elif [ "$state" = "success" ]; then - echo ":tada: **${{ matrix.name }}**: tests **passed** -- the expected-failures carve-out for this job can be retired" >> "$GITHUB_STEP_SUMMARY" - else - echo "::warning::${{ matrix.name }} job hit an infrastructure error before the tests step completed (outcome: ${{ steps.tests.outcome }})" - echo ":x: **${{ matrix.name }}**: job hit an **infrastructure error** before the tests step completed (outcome: ${{ steps.tests.outcome }})" >> "$GITHUB_STEP_SUMMARY" - fi - - name: Upload test outcome - if: always() - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # 7.0.1 - with: - # Include the matrix entry name so the artifact names stay unique if - # this matrix grows (upload-artifact v4+ errors on duplicate names). - name: strict-job-outcome-${{ matrix.name }} - path: strict-outcome.json diff --git a/.github/workflows/strict_status.yml b/.github/workflows/strict_status.yml deleted file mode 100644 index fa67f3cc..00000000 --- a/.github/workflows/strict_status.yml +++ /dev/null @@ -1,147 +0,0 @@ -# Turn the outcome of the expected-failures strict array-API jobs into -# commit statuses on the PR head. This runs as a separate workflow_run -# workflow because pull_request runs for fork PRs get a read-only -# GITHUB_TOKEN and cannot post statuses themselves. -# -# Commit statuses are used rather than check runs because check runs -# created with the Actions token get attached to the oldest github-actions -# check suite for the SHA, and the PR merge box renders only the newest -# suite per workflow -- so on any PR with more than one CI run (any push or -# close/reopen) the check vanished from the merge box. Statuses attach -# directly to the SHA and always render there. -# -# Each status uses a stable context ("strict-status/" plus the matrix entry -# name); the outcome is carried by the state and description. An expected -# test failure is reported as success (the statuses API has no neutral -# state) with a description saying so, so the PR rollup stays green. These -# statuses are informational only and are not intended to be required -# checks while the failures are expected. -# -# Note: workflow_run workflows execute from the default branch, so changes to -# this file only take effect once merged. -name: Strict array API status - -on: - workflow_run: - workflows: ["CI"] - types: [completed] - -permissions: - statuses: write - actions: read - -jobs: - report: - # Only report on pull_request runs. CI also triggers on push (all - # branches) and schedule, so without this gate a same-repo PR branch gets - # two completed CI runs per push and this workflow would post duplicate - # (possibly contradictory) checks on the same head SHA. - if: github.event.workflow_run.event == 'pull_request' - runs-on: ubuntu-latest - steps: - - name: Download strict job outcomes - id: download - # The artifacts are missing for CI runs from before the record/upload - # steps became `if: always()` (or when the whole CI run was skipped); - # tolerate that and skip the check runs below. Runs after that change - # should always upload an outcome artifact per matrix entry. - continue-on-error: true - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # 8.0.1 - with: - pattern: strict-job-outcome-* - merge-multiple: false - path: strict-outcomes - run-id: ${{ github.event.workflow_run.id }} - github-token: ${{ secrets.GITHUB_TOKEN }} - - name: Post commit statuses - if: steps.download.outcome == 'success' - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 - with: - script: | - const fs = require('fs'); - const path = require('path'); - const run = context.payload.workflow_run; - const root = 'strict-outcomes'; - // One artifact per expected-failures matrix entry, each holding a - // strict-outcome.json written by ci_tests.yml with the schema - // {"name": "", "state": "success"|"failure"|"error"}. - // download-artifact extracts each artifact into its own - // strict-job-outcome-* subdirectory when the pattern matches - // several artifacts, but flattens a single match directly into the - // root -- scan for the JSON files so both layouts work. - let entries = []; - try { - entries = fs.readdirSync(root, { withFileTypes: true }); - } catch (err) { - core.info(`No outcome artifacts downloaded (${err.message}); nothing to report.`); - return; - } - const outcomes = []; - if (entries.some((e) => e.isFile() && e.name === 'strict-outcome.json')) { - outcomes.push({ - jsonPath: path.join(root, 'strict-outcome.json'), - // The flat layout loses the artifact name; the JSON inside - // normally supplies the real one. - fallbackName: 'strict-job', - }); - } - for (const entry of entries) { - if (entry.isDirectory() && entry.name.startsWith('strict-job-outcome-')) { - outcomes.push({ - jsonPath: path.join(root, entry.name, 'strict-outcome.json'), - fallbackName: entry.name.replace('strict-job-outcome-', ''), - }); - } - } - if (outcomes.length === 0) { - core.info('No strict-outcome.json files found in downloaded artifacts; nothing to report.'); - return; - } - for (const { jsonPath, fallbackName } of outcomes) { - // Fall back to the artifact name and an error state if the JSON - // is missing or malformed, so a broken upload still surfaces. - let name = fallbackName; - let state = 'error'; - try { - const data = JSON.parse(fs.readFileSync(jsonPath, 'utf8')); - if (typeof data.name === 'string' && data.name) { - name = data.name; - } - if (['success', 'failure', 'error'].includes(data.state)) { - state = data.state; - } else { - core.warning(`Unrecognized state ${JSON.stringify(data.state)} in ${jsonPath}; reporting an error state.`); - } - } catch (err) { - core.warning(`Could not read outcome JSON at ${jsonPath} (${err.message}); reporting an error state.`); - } - // Descriptions must stay under the statuses API's 140-character - // limit; the target_url links to the CI run for details. - let statusState; - let description; - if (state === 'success') { - statusState = 'success'; - description = 'Strict array-API tests passed; ' + - 'the expected-failures carve-out for this job can be retired.'; - } else if (state === 'failure') { - statusState = 'success'; - description = 'Strict array-API tests failed (expected until ' + - 'the remaining array-API bugs are fixed).'; - } else { - statusState = 'error'; - description = 'Strict job did not complete its tests step; ' + - 'infrastructure error, not a test failure.'; - } - // Stable context; the outcome lives in the state and - // description so the merge box does not accumulate - // differently-named statuses per outcome. - await github.rest.repos.createCommitStatus({ - owner: context.repo.owner, - repo: context.repo.repo, - sha: run.head_sha, - state: statusState, - target_url: run.html_url, - description, - context: `strict-status/${name}`, - }); - } diff --git a/CHANGES.rst b/CHANGES.rst index 011af9ce..bf1061fc 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -20,6 +20,11 @@ New Features Other Changes and Additions ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +- The ``py313-strict`` CI job (the ``array-api-strict`` suite on a + non-default device) is now part of the regular CI matrix and fails CI on + a test failure; the expected-failures carve-out and the + ``strict_status.yml`` reporting workflow are retired now that the suite + is green on that backend. [#1005] - Add triage tooling for array-API backend failures: ``backend_xfail`` / ``backend_skip`` markers, an escape-site report (``CCDPROC_TRIAGE_ESCAPES=1``), and an escape logger From 26fdc63cafb212fe4409fb175020e15f46b2aa34 Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Sun, 30 Aug 2026 18:45:50 -0500 Subject: [PATCH 3/4] Address review on #1005: scope the rebin xfail, shorten changelog entries Move the backend_xfail on test_rebin_ccddata into pytest.param marks for the (True, True) case only, so a regression in the passing (False, False) case is not swallowed by the non-strict xfail (Copilot's catch). Shorten the two changelog entries as requested. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01LVxnTLrWKStBdPwcmDNhxA --- CHANGES.rst | 15 ++++++--------- ccdproc/tests/test_rebin.py | 21 +++++++++++++++------ 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index bf1061fc..7a6f1399 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -20,11 +20,9 @@ New Features Other Changes and Additions ^^^^^^^^^^^^^^^^^^^^^^^^^^^ -- The ``py313-strict`` CI job (the ``array-api-strict`` suite on a - non-default device) is now part of the regular CI matrix and fails CI on - a test failure; the expected-failures carve-out and the - ``strict_status.yml`` reporting workflow are retired now that the suite - is green on that backend. [#1005] +- The ``py313-strict`` CI job is now part of the regular CI matrix and fails + CI on a test failure; the expected-failures carve-out and the + ``strict_status.yml`` reporting workflow are retired. [#1005] - Add triage tooling for array-API backend failures: ``backend_xfail`` / ``backend_skip`` markers, an escape-site report (``CCDPROC_TRIAGE_ESCAPES=1``), and an escape logger @@ -42,10 +40,9 @@ Other Changes and Additions Bug Fixes ^^^^^^^^^ -- ``flat_correct`` now checks whether the flat has any masked pixels with - ``xp.any`` instead of the ``mask.any()`` method, which arrays of - spec-only array-API namespaces such as ``array-api-strict`` do not - have. [#1005] +- ``flat_correct`` now checks the flat's mask with ``xp.any`` instead of the + ``mask.any()`` method, which spec-only array-API namespaces do not + provide. [#1005] - Exclude masked and clipped pixels, and their weights, when computing weighted average combinations. [#952] - ``cosmicray_median`` now honors the mask of a ``CCDData`` or masked-array diff --git a/ccdproc/tests/test_rebin.py b/ccdproc/tests/test_rebin.py index 65d8f5bd..41d4d3ff 100644 --- a/ccdproc/tests/test_rebin.py +++ b/ccdproc/tests/test_rebin.py @@ -64,13 +64,22 @@ def test_rebin_smaller(): # test rebinning with ccddata object -@pytest.mark.backend_xfail( - "array-api-strict", - reason="astropy's CCDData.mask setter coerces the mask with " - "np.asarray, which fails for an array-api-strict array on the " - "non-default device", +@pytest.mark.parametrize( + "mask_data, uncertainty", + [ + (False, False), + pytest.param( + True, + True, + marks=pytest.mark.backend_xfail( + "array-api-strict", + reason="astropy's CCDData.mask setter coerces the mask with " + "np.asarray, which fails for an array-api-strict array on " + "the non-default device", + ), + ), + ], ) -@pytest.mark.parametrize("mask_data, uncertainty", [(False, False), (True, True)]) def test_rebin_ccddata(mask_data, uncertainty): ccd_data = ccd_data_func(data_size=10) if mask_data: From fd5aab81f3900780c42a4feac81e9c89c3fa7f53 Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Sun, 30 Aug 2026 18:51:53 -0500 Subject: [PATCH 4/4] Rewrite the stale strict-CI-signal docs section for the promoted job The 'Reading the strict CI signal' section in docs/array_api.rst still described the continue-on-error carve-out and the strict_status.yml check run, both removed by this PR; describe the promoted regular matrix job instead. Found by an adversarial review of the PR. Also make the changelog wording precise: it is the arrays of spec-only namespaces that lack the .any() method, not the namespaces. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01LVxnTLrWKStBdPwcmDNhxA --- CHANGES.rst | 4 ++-- docs/array_api.rst | 22 ++++++---------------- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 7a6f1399..2790d045 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -41,8 +41,8 @@ Bug Fixes ^^^^^^^^^ - ``flat_correct`` now checks the flat's mask with ``xp.any`` instead of the - ``mask.any()`` method, which spec-only array-API namespaces do not - provide. [#1005] + ``mask.any()`` method, which arrays of spec-only array-API namespaces do + not have. [#1005] - Exclude masked and clipped pixels, and their weights, when computing weighted average combinations. [#952] - ``cosmicray_median`` now honors the mask of a ``CCDData`` or masked-array diff --git a/docs/array_api.rst b/docs/array_api.rst index ba435c41..1c7c37fd 100644 --- a/docs/array_api.rst +++ b/docs/array_api.rst @@ -129,22 +129,12 @@ Reading the strict CI signal ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The ``ubuntu-py313-strict`` job in the main CI matrix runs the test suite -against `array-api-strict`_, but it is marked ``continue-on-error``, so it -always shows green in the pull-request checks rollup regardless of the test -outcome. The real result is posted as a separate check run, also named -``ubuntu-py313-strict``, by the "Strict array API status" workflow. Its -conclusion is: - -+ ``success`` -- the strict test suite passed; -+ ``neutral``, with a "failing (expected)" message -- the strict suite - failed, which is expected while known array-API bugs remain; -+ ``failure`` -- the job itself broke (an infrastructure problem rather - than the expected test failures). - -Because that workflow is triggered by ``workflow_run``, it executes from the -repository's default branch: the separate check only appears once -the workflow file exists on the default branch, and changes to it take -effect only after they are merged. +against `array-api-strict`_ on a non-default device, a CPU-only proxy for +GPU-style device behavior. It is a regular matrix job: a test failure fails +the job and the pull-request checks rollup, like any other backend. A +failure there usually means a numpy-ism (a numpy-only method or type, a +missing ``device=``) crept into a code path that the more permissive +backends accept silently; reproduce it locally with ``tox -e strict``. What limitations should I be aware of? ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~