diff --git a/.github/workflows/ci_tests.yml b/.github/workflows/ci_tests.yml index 3667baa4..5a1be706 100644 --- a/.github/workflows/ci_tests.yml +++ b/.github/workflows/ci_tests.yml @@ -58,6 +58,16 @@ jobs: python: '3.12' tox_env: 'py312-alldeps-dask' + # Regression gate: fail if a new array-API "escape" (a silent numpy + # coercion of a dask array in library code) appears that is not in + # the checked-in baseline, ccdproc/tests/array_escape_baseline.txt. + # No bottleneck here (as with the dask job above) so the numpy + # fallback paths are exercised. + - name: 'ubuntu-py312-dask-escape-baseline' + os: ubuntu-latest + python: '3.12' + tox_env: 'py312-alldeps-dask-enforce' + - name: 'windows-py312' os: windows-latest python: '3.12' @@ -108,3 +118,77 @@ jobs: - name: Upload coverage to codecov if: "endsWith(matrix.tox_env, '-cov')" uses: codecov/codecov-action@671740ac38dd9b0130fbe1cec585b89eea48d3de # v5.5.2 + + # 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@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + with: + fetch-depth: 0 + - name: Set up Python ${{ matrix.python }} + uses: actions/setup-python@83679a892e2d95755f2dac6acb0bfd1e9ac5d548 # v6.1.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@b7c566a772e6b6bfb58ed0dc250532a479d7789f # 6.0.0 + 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 new file mode 100644 index 00000000..4ded2a0f --- /dev/null +++ b/.github/workflows/strict_status.yml @@ -0,0 +1,121 @@ +# Turn the outcome of the expected-failures strict array-API jobs into +# check runs on the PR. This runs as a separate workflow_run workflow because +# pull_request runs for fork PRs get a read-only GITHUB_TOKEN and cannot +# create check runs themselves. Each check uses a stable name (the matrix +# entry name); the outcome is carried by the conclusion and output instead. +# These checks are informational only (an expected test failure is reported +# as neutral, so the PR rollup stays green) 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: + checks: 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@37930b1c2abaa49bbe596cd826c3c89aef350131 # 7.0.0 + with: + pattern: strict-job-outcome-* + merge-multiple: false + path: strict-outcomes + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ secrets.GITHUB_TOKEN }} + - name: Create check runs + 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 directory per artifact, i.e. per expected-failures matrix + // entry. Each contains a strict-outcome.json written by ci_tests.yml + // with the schema {"name": "", + // "state": "success" | "failure" | "error"}. + let dirs = []; + try { + dirs = fs.readdirSync(root).filter((d) => d.startsWith('strict-job-outcome-')); + } catch (err) { + core.info(`No outcome artifacts downloaded (${err.message}); nothing to report.`); + return; + } + if (dirs.length === 0) { + core.info('No strict-job-outcome-* artifacts found; nothing to report.'); + return; + } + for (const dir of dirs) { + // 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 = dir.replace('strict-job-outcome-', ''); + let state = 'error'; + try { + const data = JSON.parse(fs.readFileSync(path.join(root, dir, 'strict-outcome.json'), '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 ${dir}; reporting an error state.`); + } + } catch (err) { + core.warning(`Could not read outcome JSON in ${dir} (${err.message}); reporting an error state.`); + } + let conclusion; + let title; + let summary; + if (state === 'success') { + conclusion = 'success'; + title = 'Strict array-API tests passed'; + summary = `The ${name} job in [this CI run](${run.html_url}) passed. ` + + 'The expected-failures carve-out for it can be retired.'; + } else if (state === 'failure') { + conclusion = 'neutral'; + title = 'Strict array-API tests failed (expected)'; + summary = `The ${name} job in [this CI run](${run.html_url}) failed. ` + + 'This is expected until the remaining array-API bugs are fixed; ' + + 'see the run log for details.'; + } else { + conclusion = 'failure'; + title = 'Strict array-API job broke (infra error)'; + summary = `The ${name} job in [this CI run](${run.html_url}) did not ` + + 'complete its tests step. This is an infrastructure error in the ' + + 'job itself, not a test failure; see the run log for details.'; + } + // Stable check name (just the matrix entry name); the outcome + // lives in the conclusion and output so the checks UI does not + // accumulate differently-named checks per outcome. + await github.rest.checks.create({ + owner: context.repo.owner, + repo: context.repo.repo, + name, + head_sha: run.head_sha, + status: 'completed', + conclusion, + output: { title, summary }, + }); + } diff --git a/CHANGES.rst b/CHANGES.rst index 3b571709..6e3e2da0 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,3 +1,16 @@ +2.6.0 (unreleased) +------------------ + +New Features +^^^^^^^^^^^^ + +- Add ``array-api-strict`` (on a non-default device) as a test backend, a + CPU-only proxy for CuPy device behavior. [#942] +- 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 + (``CCDPROC_LOG_ARRAY_ESCAPES=1``). [#942] + 2.5.1 (2025-07-05) ------------------ diff --git a/ccdproc/conftest.py b/ccdproc/conftest.py index 7806f8d5..c5c2ac3f 100644 --- a/ccdproc/conftest.py +++ b/ccdproc/conftest.py @@ -3,9 +3,14 @@ # this contains imports plugins that configure py.test for astropy tests. # by importing them here in conftest.py they are discoverable by py.test # no matter how it is invoked within the source tree. +import logging import os +import traceback -import array_api_compat # noqa: F401 +import array_api_compat +import array_api_compat.numpy +import numpy as np +import pytest try: # When the pytest_astropy_header package is installed @@ -19,6 +24,14 @@ def pytest_configure(config): TESTED_VERSIONS = {} +from .tests._escape_triage import ( + _env_truthy, + pytest_runtest_makereport, # noqa: F401 pytest hook, used via attribute lookup + pytest_sessionfinish, # noqa: F401 pytest hook, used via attribute lookup + pytest_sessionstart, # noqa: F401 pytest hook, used via attribute lookup + pytest_terminal_summary, # noqa: F401 pytest hook, used via attribute lookup + record_escape_log, +) from .tests.pytest_fixtures import ( triage_setup, # noqa: F401 this is used in tests ) @@ -41,6 +54,16 @@ def pytest_configure(config): # What happens here is controlled by an environmental variable array_library = os.environ.get("CCDPROC_ARRAY_LIBRARY", "numpy").lower() +# Device to create test arrays on. This is only meaningful for backends that +# support multiple devices (currently array-api-strict, as a CPU-only proxy +# for testing non-default-device behavior like CuPy's GPU device). Leaving +# CCDPROC_ARRAY_DEVICE unset selects the backend's testing default: the +# non-default "device1" for array-api-strict, None (the library's usual +# device) for everything else. Setting it to "default" selects the library's +# normal default device; any other value is passed to the library's Device +# constructor. +testing_array_device = None + match array_library: case "numpy": import array_api_compat.numpy as testing_array_library # noqa: F401 @@ -60,8 +83,248 @@ def pytest_configure(config): PYTEST_HEADER_MODULES["cupy"] = "cupy" + case "array-api-strict" | "array_api_strict": + import array_api_strict as testing_array_library # noqa: F401 + + PYTEST_HEADER_MODULES["array_api_strict"] = "array_api_strict" + + # array-api-strict exposes a couple of extra fake devices in addition + # to its default CPU_DEVICE. Using one of those non-default devices + # here makes np.asarray() raise on the resulting arrays, the same way + # it would for a CuPy array living on a GPU. That makes + # array-api-strict a convenient CPU-only proxy for catching the + # "silent conversion to numpy" bugs that would otherwise only show up + # on CuPy. + device_name = os.environ.get("CCDPROC_ARRAY_DEVICE", "device1") + if device_name.lower() == "default": + # The library's normal CPU device, on which np.asarray() succeeds. + testing_array_device = testing_array_library.Device("CPU_DEVICE") + else: + testing_array_device = testing_array_library.Device(device_name) + case _: raise ValueError( f"Unsupported array library: {array_library}. " "Supported libraries are listed at https://ccdproc.readthedocs.io/en/latest/array_api.html." ) + + +# --------------------------------------------------------------------------- +# Per-backend xfail/skip markers +# +# @pytest.mark.backend_xfail("cupy", "array-api-strict", reason="...") +# @pytest.mark.backend_skip("cupy", reason="...") +# +# These let individual tests be marked as expected-failures or skips only +# when run against specific array backends (as set by CCDPROC_ARRAY_LIBRARY), +# without affecting the default numpy-backed test run. +# --------------------------------------------------------------------------- + + +def _normalize_backend_name(name): + return name.lower().replace("_", "-") + + +_ACTIVE_BACKEND = _normalize_backend_name(array_library) + + +def pytest_collection_modifyitems(items): + """ + Apply the ``backend_skip`` / ``backend_xfail`` markers at collection + time. For each collected test, the backends named in a marker's + positional args (normalized, so ``array_api_strict`` and + ``array-api-strict`` match) are compared against the active backend from + ``CCDPROC_ARRAY_LIBRARY``; on a match the corresponding standard pytest + marker is attached -- a plain skip, or a *non-strict* xfail so a test + that starts passing XPASSes instead of failing the run (see the "prune + on XPASS" note in docs/array_api.rst). Tests without a matching marker, + including everything in the default numpy run, are untouched. + """ + for item in items: + for marker in item.iter_markers(name="backend_skip"): + backends = {_normalize_backend_name(b) for b in marker.args} + if _ACTIVE_BACKEND in backends: + reason = marker.kwargs.get( + "reason", f"skipped for array backend {_ACTIVE_BACKEND!r}" + ) + item.add_marker(pytest.mark.skip(reason=reason)) + # One matching marker decides the outcome; skip the rest. + break + + for marker in item.iter_markers(name="backend_xfail"): + backends = {_normalize_backend_name(b) for b in marker.args} + if _ACTIVE_BACKEND in backends: + reason = marker.kwargs.get( + "reason", f"expected failure for array backend {_ACTIVE_BACKEND!r}" + ) + item.add_marker(pytest.mark.xfail(reason=reason, strict=False)) + break + + +# --------------------------------------------------------------------------- +# Escape logger: catch silent conversion of non-numpy array-API arrays back +# to numpy. On CuPy such a conversion typically raises immediately (because +# the array lives on a GPU), which is how those bugs are usually found. On +# backends like dask and jax the conversion often succeeds silently, so the +# bug is easy to miss. This monkeypatches np.asarray/np.asanyarray (and +# np.ma.asanyarray) to log a warning, when active, any time they are handed +# an array whose array-API namespace (as resolved by +# array_api_compat.array_namespace) is not numpy. +# +# Only Python-level calls through the module attributes are visible: +# C-level coercions inside compiled dependencies (scipy, astroscrappy, +# reproject) and references bound before the patch ("from numpy import +# asarray") bypass the wrappers entirely, so absence of a warning is not +# proof there was no conversion. +# +# Activated by setting the environment variable CCDPROC_LOG_ARRAY_ESCAPES to +# a truthy value. +# --------------------------------------------------------------------------- + +_escape_logger = logging.getLogger("ccdproc.array_escape") + +_LOG_ARRAY_ESCAPES = _env_truthy(os.environ.get("CCDPROC_LOG_ARRAY_ESCAPES", "")) + +# array_api_compat.numpy wraps plain numpy; arrays built through it report +# __array_namespace__() as one of these two modules. Neither counts as an +# "escape" -- we only care about arrays from a genuinely different library +# (jax, dask, cupy, array_api_strict, ...) ending up in a numpy-only call. +_NUMPY_LIKE_NAMESPACES = {np, array_api_compat.numpy} + + +def _foreign_namespace(obj): + """ + Return obj's array-API namespace if it is foreign, else None. + + numpy arrays (including np.ma masked arrays) are never foreign. + Detection goes through array_api_compat.array_namespace() rather than + the raw __array_namespace__ dunder because some backends (notably dask) + do not define the dunder on their array objects even though + array-api-compat can resolve a namespace for them. The namespace is + foreign unless it is numpy or array_api_compat's numpy wrapper -- i.e. + exactly the arrays whose conversion to numpy would fail (CuPy on GPU) + or silently densify/transfer (dask, jax). The try/except guards against + non-arrays and misbehaving objects (including unhashable namespaces); + those are treated as not foreign so the logger never breaks the call + it wraps. + """ + if isinstance(obj, np.ndarray): + return None + try: + namespace = array_api_compat.array_namespace(obj) + if namespace in _NUMPY_LIKE_NAMESPACES: + return None + except Exception: + return None + return namespace + + +def _escape_site_frame(): + """ + Return the FrameSummary for the innermost stack frame that is inside + ccdproc but not inside ccdproc's own test suite -- the frame blamed for + an escape -- or None if there is no such frame. Frames in this module are + classified as test frames and are never chosen, so the exact call depth + here does not affect the result. + """ + from .tests._escape_triage import locate_escape_site + + return locate_escape_site(traceback.extract_stack()) + + +def _describe_escape_site(frame): + """Short "file:line function" string for an escape log message.""" + if frame is None: + return "" + return f"{frame.filename}:{frame.lineno} {frame.name}" + + +class _ReentrancyGuard: + """Small helper to keep our wrappers from recursing into themselves.""" + + def __init__(self): + self.active = False + + +def _make_escape_logging_wrapper(original, funcname, guard): + """ + Build the replacement for one numpy coercion entry point (``original`` is + the real ``np.asarray``, ``np.asanyarray`` or ``np.ma.asanyarray``). + The wrapper logs and tallies an "array-API escape" whenever its first + positional argument is an array from a non-numpy array-API library, then + always finishes by calling ``original`` -- behavior is unchanged, escapes + are only observed. ``funcname`` is the dotted numpy name (e.g. + ``numpy.asarray``) recorded with each escape. ``guard`` breaks recursion: + the namespace detection and stack extraction below can themselves end up + calling the patched functions. + """ + + def wrapper(*args, **kwargs): + # Do nothing extra on re-entrant calls (guard held) or when there is + # no positional argument to inspect. + if not guard.active and args: + guard.active = True + try: + obj = args[0] + namespace = _foreign_namespace(obj) + if namespace is not None: + # A foreign array is about to be coerced to numpy: warn + # with the innermost non-test ccdproc frame to blame, and + # tally the site for the end-of-session summary and the + # baseline ratchet. + frame = _escape_site_frame() + site = _describe_escape_site(frame) + _escape_logger.warning( + "array-API escape: %s() called on a %r array " + "(namespace=%r) at %s", + funcname, + type(obj), + namespace, + site, + ) + record_escape_log(frame, funcname) + finally: + guard.active = False + return original(*args, **kwargs) + + wrapper.__name__ = getattr(original, "__name__", funcname) + wrapper.__doc__ = getattr(original, "__doc__", None) + return wrapper + + +@pytest.fixture(autouse=True, scope="session") +def _log_array_escapes(): + """ + Session-scoped autouse fixture that, when CCDPROC_LOG_ARRAY_ESCAPES is + set, monkeypatches numpy's array-coercion entry points for the duration + of the test session so that any silent conversion of a non-numpy + array-API array is logged instead of passing unnoticed. + """ + if not _LOG_ARRAY_ESCAPES: + yield + return + + guard = _ReentrancyGuard() + + originals = { + "np.asarray": np.asarray, + "np.asanyarray": np.asanyarray, + "np.ma.asanyarray": np.ma.asanyarray, + } + + np.asarray = _make_escape_logging_wrapper( + originals["np.asarray"], "numpy.asarray", guard + ) + np.asanyarray = _make_escape_logging_wrapper( + originals["np.asanyarray"], "numpy.asanyarray", guard + ) + np.ma.asanyarray = _make_escape_logging_wrapper( + originals["np.ma.asanyarray"], "numpy.ma.asanyarray", guard + ) + + try: + yield + finally: + np.asarray = originals["np.asarray"] + np.asanyarray = originals["np.asanyarray"] + np.ma.asanyarray = originals["np.ma.asanyarray"] diff --git a/ccdproc/tests/_escape_triage.py b/ccdproc/tests/_escape_triage.py new file mode 100644 index 00000000..973d48f7 --- /dev/null +++ b/ccdproc/tests/_escape_triage.py @@ -0,0 +1,536 @@ +# Licensed under a 3-clause BSD style license - see LICENSE.rst +""" +Pytest hooks used to triage array-API "escape" failures: cases where a +non-numpy array unexpectedly gets silently converted to (or otherwise +touches) plain numpy deep inside ccdproc. Such conversions are usually +harmless on numpy itself, but will raise loudly on backends like CuPy +(because the array lives on a GPU) and are the kind of bug this tooling is +meant to help find on other backends too. + +The escape logger only sees Python-level module-attribute calls to +``np.asarray``/``np.asanyarray``/``np.ma.asanyarray`` (they are +monkeypatched in ``ccdproc/conftest.py``). C-level coercions inside +compiled dependencies (scipy, astroscrappy, reproject) and names bound +before the patch (``from numpy import asarray``) are invisible to it -- +structural false negatives, in the logger, the baseline and the ratchet. + +Activated by setting the environment variable ``CCDPROC_TRIAGE_ESCAPES`` to +a truthy value ("1", "true", "yes", "on", case-insensitive). When active, +every test failure's traceback is inspected to find the "escape site": the +innermost frame that is inside the ``ccdproc`` package but *not* inside +``ccdproc``'s own test suite (``ccdproc/tests``). If no such frame is found, +falls back to the innermost ``ccdproc`` frame, and finally to the innermost +frame overall. Failures are grouped by that site and a summary is printed at +the end of the test session (most frequent site first). This is meant to +replace manually eyeballing ``--tb=long`` output to find patterns across a +batch of failures. +""" + +import os +import traceback +from collections import defaultdict + +import pytest + +_TRUTHY = {"1", "true", "yes", "on"} + + +def _env_truthy(value): + return str(value).strip().lower() in _TRUTHY + + +TRIAGE_ACTIVE = _env_truthy(os.environ.get("CCDPROC_TRIAGE_ESCAPES", "")) + +#: When set, compare the live-logged library escape sites against the +#: checked-in baseline and fail the session if any *new* site appears. +ENFORCE_BASELINE = _env_truthy(os.environ.get("CCDPROC_ENFORCE_ESCAPE_BASELINE", "")) + +#: When set, (re)write the baseline file from the escapes observed this run +#: instead of enforcing it. Use to seed or refresh the baseline. +WRITE_BASELINE = _env_truthy(os.environ.get("CCDPROC_WRITE_ESCAPE_BASELINE", "")) + +#: Whether the live escape logger in ``conftest.py`` is active. Both baseline +#: modes read the sites the logger tallies into ``_ESCAPE_LOG_COUNTS``, so +#: neither can do anything meaningful without it (see pytest_sessionstart). +LOG_ESCAPES = _env_truthy(os.environ.get("CCDPROC_LOG_ARRAY_ESCAPES", "")) + +#: Maps a (filename, lineno, function) escape site to a list of test node +#: ids that failed with that site as their innermost non-test ccdproc frame. +_ESCAPE_SITES = defaultdict(list) + +#: Maps a (relfile, lineno, function, funcname) escape key -- the +#: package-relative file, line and function of the innermost frame the live +#: logger blamed, plus the numpy entry point (e.g. "numpy.asarray") that +#: performed the coercion -- to the number of times that conversion was +#: logged during the session. Populated by the live escape logger in +#: ``conftest.py`` (only when ``CCDPROC_LOG_ARRAY_ESCAPES`` is active). +#: Unlike ``_ESCAPE_SITES`` these escapes do not fail the test -- on dask/jax +#: the conversion succeeds silently -- so the tally is the only way to +#: collapse the streamed warnings into a summary, and it is the observed-set +#: input to the baseline ratchet. +_ESCAPE_LOG_COUNTS = defaultdict(int) + + +def record_escape_log(frame, funcname): + """ + Tally one live-logged escape for the end-of-session summary and the + baseline ratchet. ``frame`` is the FrameSummary chosen by + ``locate_escape_site()`` (or None if no frame could be found). + """ + if frame is None: + key = ("", 0, "", funcname) + else: + key = (_relpath(frame.filename), frame.lineno, frame.name, funcname) + _ESCAPE_LOG_COUNTS[key] += 1 + + +# Anchor frame classification on this file's actual location rather than +# looking for "ccdproc" anywhere in the path: on CI the repository checkout +# directory is itself named ccdproc, so a substring test would classify every +# frame (including tox's site-packages) as a ccdproc frame. +_TESTS_ROOT = os.path.dirname(os.path.abspath(__file__)) + os.sep +_PACKAGE_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + os.sep +_CONFTEST_PATH = os.path.join(_PACKAGE_ROOT, "conftest.py") + + +def _is_ccdproc_frame(filename): + return os.path.abspath(filename).startswith(_PACKAGE_ROOT) + + +def _is_ccdproc_test_frame(filename): + # ccdproc/conftest.py hosts the escape-logger wrapper and other test + # infrastructure, so it counts as a test frame: it must never be + # reported as the escape site. + abspath = os.path.abspath(filename) + return abspath.startswith(_TESTS_ROOT) or abspath == _CONFTEST_PATH + + +#: Checked-in list of known library escape sites (the ratchet baseline). +_BASELINE_PATH = os.path.join( + os.path.dirname(os.path.abspath(__file__)), "array_escape_baseline.txt" +) + + +def _relpath(filename): + """Package-relative, forward-slash path, for stable baseline keys.""" + try: + rel = os.path.relpath(os.path.abspath(filename), _PACKAGE_ROOT) + except ValueError: # e.g. a different drive on Windows + rel = os.path.abspath(filename) + return rel.replace(os.sep, "/") + + +def _is_library_site(relfile): + """ + True for escapes blamed on real ccdproc library code -- i.e. not the test + suite, conftest, or an unknown location. Only library sites go into the + baseline ratchet: an escape blamed on a test frame is not an actionable + migration target. + """ + if relfile == "": + return False + abspath = os.path.join(_PACKAGE_ROOT, relfile.replace("/", os.sep)) + return _is_ccdproc_frame(abspath) and not _is_ccdproc_test_frame(abspath) + + +def _observed_library_sites(): + """Set of (relfile, function, coercion) for the library escapes seen.""" + return { + (relfile, function, funcname) + for (relfile, lineno, function, funcname) in _ESCAPE_LOG_COUNTS + if _is_library_site(relfile) + } + + +def _load_escape_baseline(): + """ + Parse the baseline file into ``{(relfile, function, coercion): reason}``. + + Blank lines and ``#`` comments are ignored. Each entry is whitespace + separated: the first three tokens are the file, function and coercion + (none of which contain spaces); anything after is a free-text reason/tag + for humans and is ignored by the ratchet. + """ + baseline = {} + try: + with open(_BASELINE_PATH, encoding="utf-8") as f: + raw_lines = f.readlines() + except FileNotFoundError: + return baseline + for raw in raw_lines: + line = raw.strip() + if not line or line.startswith("#"): + continue + parts = line.split(None, 3) + if len(parts) < 3: + continue + key = (parts[0], parts[1], parts[2]) + baseline[key] = parts[3] if len(parts) > 3 else "" + return baseline + + +def _new_escapes(): + """Library escapes observed this run that are absent from the baseline.""" + return sorted(_observed_library_sites() - set(_load_escape_baseline())) + + +def _stale_baseline_entries(): + """Baseline entries not hit this run (candidates for deletion).""" + return sorted(set(_load_escape_baseline()) - _observed_library_sites()) + + +#: Baseline entries dropped by the last _write_escape_baseline() call -- +#: entries in the old file that were not observed this run. Stashed so the +#: terminal summary can make the shrink visible (a subset run drops entries +#: for code it simply never exercised). +_BASELINE_DROPPED = [] + + +def _write_escape_baseline(): + """ + (Re)write the baseline file from the library escapes observed this run. + Reasons/tags already present in the file are preserved for sites that are + still observed, so hand-annotations survive a refresh. Refuses to write + (raises UsageError) if no library escapes were observed at all: that + means the run could not have exercised the escapes (numpy backend, or a + subset run that hits none) and writing would truncate the baseline. + """ + sites = sorted(_observed_library_sites()) + if not sites: + raise pytest.UsageError( + "CCDPROC_WRITE_ESCAPE_BASELINE=1: no library escapes were " + "observed this run, refusing to truncate " + f"{_relpath(_BASELINE_PATH)}. Regenerate the baseline with " + "CCDPROC_LOG_ARRAY_ESCAPES=1, a non-numpy CCDPROC_ARRAY_LIBRARY " + "(e.g. dask), and a full-suite run." + ) + existing = _load_escape_baseline() + _BASELINE_DROPPED[:] = sorted(set(existing) - set(sites)) + header = [ + "# Array-API escape baseline for non-numpy backends (dask/jax).", + "# Columns: ", + "#", + "# The ratchet (CCDPROC_ENFORCE_ESCAPE_BASELINE=1) fails the session", + "# if a library escape appears that is not listed here. Delete an", + "# entry as you migrate that call site; this file only shrinks.", + "# Regenerate with CCDPROC_WRITE_ESCAPE_BASELINE=1 (preserves tags);", + "# that requires CCDPROC_LOG_ARRAY_ESCAPES=1, a non-numpy", + "# CCDPROC_ARRAY_LIBRARY (e.g. dask), and a *full* test-suite run --", + "# a subset run drops the entries its tests never exercise.", + "#", + "# Tags are for humans, not the ratchet: TODO = still to migrate,", + "# BOUNDARY = a numpy-only dependency (scipy/astroscrappy/reproject)", + "# that will never leave. Verify/adjust the seeded tags by hand.", + "#", + ] + # Per-column widths for aligned output; sites is non-empty here (the + # empty-observation case raised above), so max() cannot see an empty + # sequence. + w_file, w_func, w_co = ( + max(len(s) for s in col) for col in zip(*sites, strict=True) + ) + body = [] + for key in sites: + relfile, function, coercion = key + reason = existing.get(key, "TODO") + body.append( + f"{relfile:<{w_file}} {function:<{w_func}} " + f"{coercion:<{w_co}} {reason}".rstrip() + ) + _write_lines(header + body) + + +def _write_lines(lines): + with open(_BASELINE_PATH, "w", encoding="utf-8") as f: + f.write("\n".join(lines) + "\n") + + +def locate_escape_site(frames): + """ + Given an iterable of frame-summary-like objects (as returned by + ``traceback.extract_tb`` or ``traceback.extract_stack``, i.e. anything + with ``.filename``, ``.lineno`` and ``.name`` attributes), find the + frame most likely responsible for an array-API "escape". + + Preference order: + + 1. The innermost frame inside the ``ccdproc`` package that is not part + of ``ccdproc``'s own test suite. + 2. If there is no such frame, the innermost frame inside ``ccdproc`` + at all (this will typically be a test-suite frame). + 3. If there is no ``ccdproc`` frame at all, the innermost frame overall. + + Returns ``None`` if given no frames at all. + """ + frames = list(frames) + if not frames: + return None + + non_test_ccdproc = [ + f + for f in frames + if _is_ccdproc_frame(f.filename) and not _is_ccdproc_test_frame(f.filename) + ] + if non_test_ccdproc: + return non_test_ccdproc[-1] + + ccdproc_frames = [f for f in frames if _is_ccdproc_frame(f.filename)] + if ccdproc_frames: + return ccdproc_frames[-1] + + return frames[-1] + + +@pytest.hookimpl(wrapper=True) +def pytest_runtest_makereport(item, call): + """ + Record the escape site of every failing test. + + Runs as a wrapper around report generation for each test phase; only the + 'call' phase (the test body itself, not setup/teardown) of failing tests + is recorded, so an escape that raises during fixture setup is not + triaged. That is a deliberate trade-off: fixtures in this test suite are + mostly plain array/CCDData construction, so escapes surface in the test + bodies. The failure's traceback is reduced to its most informative frame + by locate_escape_site() and the test id is filed under that + (file, line, function) key for the end-of-session summary. + """ + # With a new-style wrapper, yield hands back the TestReport itself + # (not a pluggy.Result), and the wrapper must return it. + report = yield + + if ( + TRIAGE_ACTIVE + and report.when == "call" + and report.failed + and call.excinfo is not None + ): + site = locate_escape_site(traceback.extract_tb(call.excinfo.tb)) + if site is not None: + key = (site.filename, site.lineno, site.name) + _ESCAPE_SITES[key].append(item.nodeid) + + return report + + +def pytest_terminal_summary(terminalreporter): + """ + Print the array-API escape summaries at the end of the test session. + + Two independent sections, either of which may be empty: + + * the failure-triage summary (from ``_ESCAPE_SITES``), grouping test + failures by root-cause call site, and + * the live escape-log summary (from ``_ESCAPE_LOG_COUNTS``), collapsing + the streamed "array-API escape" warnings into per-site counts. + """ + _report_escape_failures(terminalreporter) + _report_escape_log_counts(terminalreporter) + _report_escape_baseline(terminalreporter) + _report_escape_baseline_dropped(terminalreporter) + + +def _report_escape_failures(terminalreporter): + """ + Print the failure-triage summary. + + One section listing each escape site with its failure count (most + common first) and up to five example test ids, so a large batch of + backend failures collapses to a short list of root-cause call sites. + """ + if not TRIAGE_ACTIVE or not _ESCAPE_SITES: + return + + terminalreporter.section("ccdproc array-API escape triage") + terminalreporter.write_line( + "Failures grouped by innermost non-test ccdproc frame " + "(file:line function), most common first:" + ) + + ordered = sorted(_ESCAPE_SITES.items(), key=lambda kv: len(kv[1]), reverse=True) + example_limit = 5 + for (filename, lineno, function), test_ids in ordered: + terminalreporter.write_line("") + terminalreporter.write_line( + f"{filename}:{lineno} {function} ({len(test_ids)} failures)" + ) + for test_id in test_ids[:example_limit]: + terminalreporter.write_line(f" - {test_id}") + if len(test_ids) > example_limit: + terminalreporter.write_line( + f" ... and {len(test_ids) - example_limit} more" + ) + + +def _report_escape_log_counts(terminalreporter): + """ + Print the live escape-log summary. + + When ``CCDPROC_LOG_ARRAY_ESCAPES`` is active the logger in ``conftest.py`` + streams one warning per silent numpy coercion of a foreign array and + tallies each call site in ``_ESCAPE_LOG_COUNTS``. These escapes do not + fail the test -- on dask/jax the conversion succeeds silently -- so this + collapses the streamed warnings into one deduplicated, most-frequent-first + list of call sites still to migrate. + """ + if not _ESCAPE_LOG_COUNTS: + return + + terminalreporter.section("ccdproc array-API escape log summary") + total = sum(_ESCAPE_LOG_COUNTS.values()) + n_sites = len(_ESCAPE_LOG_COUNTS) + terminalreporter.write_line( + f"{total} silent numpy coercion(s) of foreign arrays across " + f"{n_sites} call site(s), most frequent first:" + ) + + ordered = sorted(_ESCAPE_LOG_COUNTS.items(), key=lambda kv: kv[1], reverse=True) + for (relfile, lineno, function, funcname), count in ordered: + terminalreporter.write_line( + f" {count:>5}x {funcname}() {relfile}:{lineno} {function}" + ) + + +def _report_escape_baseline(terminalreporter): + """ + Print the baseline ratchet result: any new library escapes (which fail + the session) and any baseline entries no longer hit (safe to delete). + Only shown when enforcement is active. + """ + if not ENFORCE_BASELINE: + return + + # No foreign arrays seen at all means the logger ran but nothing tripped + # it -- a numpy backend, or a subset run that exercises no escape site. + # (Enforce mode with the logger *off* is rejected outright in + # pytest_sessionstart.) The baseline is about non-numpy backends, so + # don't report every entry as "stale" and tempt someone to delete it. + if not _observed_library_sites(): + terminalreporter.section("ccdproc array-API escape baseline") + terminalreporter.write_line( + "Escape logger was active but observed no foreign-array escapes " + "(numpy backend, or a subset run exercising no escape site); " + "baseline not checked." + ) + return + + new = _new_escapes() + stale = _stale_baseline_entries() + + terminalreporter.section("ccdproc array-API escape baseline") + if not new: + terminalreporter.write_line("OK: no library escapes outside the baseline.") + else: + terminalreporter.write_line( + f"NEW escapes not in baseline ({len(new)}) -- these fail the session:" + ) + for relfile, function, coercion in new: + terminalreporter.write_line(f" + {relfile} {function} {coercion}") + + if stale: + terminalreporter.write_line("") + terminalreporter.write_line( + f"Baseline entries not hit this run ({len(stale)}) -- delete them " + "if the migration removed the escape:" + ) + for relfile, function, coercion in stale: + terminalreporter.write_line(f" - {relfile} {function} {coercion}") + + +def _report_escape_baseline_dropped(terminalreporter): + """ + After a baseline rewrite, warn loudly about any entries the rewrite + dropped (present in the old file, not observed this run). On a full-suite + run dropping stale entries is the point of a refresh, but on a subset run + the drop just means those tests never ran -- either way it must be + visible, not a silent shrink of the ratchet. + """ + if not WRITE_BASELINE or not _BASELINE_DROPPED: + return + + terminalreporter.section("ccdproc array-API escape baseline (rewritten)") + terminalreporter.write_line( + f"WARNING: rewrite DROPPED {len(_BASELINE_DROPPED)} entr" + f"{'y' if len(_BASELINE_DROPPED) == 1 else 'ies'} present in the old " + "baseline but not observed this run:", + red=True, + bold=True, + ) + for relfile, function, coercion in _BASELINE_DROPPED: + terminalreporter.write_line(f" - {relfile} {function} {coercion}") + terminalreporter.write_line( + "If this was not a full-suite run these entries were dropped only " + "because their tests never ran -- restore the file (git checkout) " + "and regenerate from a full run." + ) + + +def _xdist_active(config): + """ + True when pytest-xdist is about to run the tests in worker processes. + ``_ESCAPE_LOG_COUNTS`` is a per-process global: under ``pytest -n`` the + workers do the tallying and the controller process (where sessionfinish + runs) sees an empty tally, so write mode would truncate the baseline and + enforce mode would pass without checking anything. + """ + if not config.pluginmanager.hasplugin("xdist"): + return False + try: + numprocesses = config.getoption("numprocesses", None) + except (KeyError, ValueError): + return False + return bool(numprocesses) + + +def pytest_sessionstart(session): + """ + Fail fast on unusable configurations. Both baseline modes read the + escape sites tallied by the live logger in ``conftest.py``, which only + runs when CCDPROC_LOG_ARRAY_ESCAPES is truthy. Without it, write mode + would observe nothing and truncate the baseline, and enforce mode would + vacuously pass -- so reject either combination before any test runs. + The same failure modes occur under pytest-xdist (see ``_xdist_active``), + so both modes are rejected there too. + """ + if (WRITE_BASELINE or ENFORCE_BASELINE) and _xdist_active(session.config): + raise pytest.UsageError( + "CCDPROC_WRITE_ESCAPE_BASELINE / CCDPROC_ENFORCE_ESCAPE_BASELINE " + "cannot run under pytest-xdist: escape tallies live in each " + "worker process and never reach the controller, so the baseline " + "would be truncated or enforcement would pass vacuously. Re-run " + "without -n (or with -n0)." + ) + if WRITE_BASELINE and not LOG_ESCAPES: + raise pytest.UsageError( + "CCDPROC_WRITE_ESCAPE_BASELINE=1 requires the escape logger: " + "without CCDPROC_LOG_ARRAY_ESCAPES=1 no escapes are observed and " + "the baseline would be wiped. Set CCDPROC_LOG_ARRAY_ESCAPES=1 and " + "a non-numpy CCDPROC_ARRAY_LIBRARY (e.g. dask) and run the full " + "test suite." + ) + if ENFORCE_BASELINE and not LOG_ESCAPES: + raise pytest.UsageError( + "CCDPROC_ENFORCE_ESCAPE_BASELINE=1 requires the escape logger: " + "without CCDPROC_LOG_ARRAY_ESCAPES=1 no escapes are observed and " + "enforcement would pass without checking anything. Set " + "CCDPROC_LOG_ARRAY_ESCAPES=1 (and a non-numpy " + "CCDPROC_ARRAY_LIBRARY, e.g. dask)." + ) + + +def pytest_sessionfinish(session, exitstatus): + """ + Baseline ratchet regeneration / enforcement. + + In write mode (CCDPROC_WRITE_ESCAPE_BASELINE) the baseline file is + rewritten from the escapes observed this run (refusing, with a + UsageError, to truncate it if nothing was observed). In enforce mode + (CCDPROC_ENFORCE_ESCAPE_BASELINE) the session exit status is forced + nonzero when a new library escape appeared, so CI fails on a regression. + A pre-existing nonzero status (real test failures) is left untouched. + """ + if WRITE_BASELINE: + _write_escape_baseline() + return + if ENFORCE_BASELINE and exitstatus == 0 and _new_escapes(): + session.exitstatus = 1 diff --git a/ccdproc/tests/array_escape_baseline.txt b/ccdproc/tests/array_escape_baseline.txt new file mode 100644 index 00000000..d6afc219 --- /dev/null +++ b/ccdproc/tests/array_escape_baseline.txt @@ -0,0 +1,25 @@ +# Array-API escape baseline for non-numpy backends (dask/jax). +# Columns: +# +# The ratchet (CCDPROC_ENFORCE_ESCAPE_BASELINE=1) fails the session +# if a library escape appears that is not listed here. Delete an +# entry as you migrate that call site; this file only shrinks. +# Regenerate with CCDPROC_WRITE_ESCAPE_BASELINE=1 (preserves tags). +# +# Tags are for humans, not the ratchet: TODO = still to migrate, +# BOUNDARY = a numpy-only dependency (scipy/astroscrappy/reproject) +# that will never leave. Verify/adjust the seeded tags by hand. +# +combiner.py average_combine numpy.asarray BOUNDARY: result stored in astropy CCDData/StdDevUncertainty (numpy-backed) +combiner.py combine numpy.asarray BOUNDARY: astropy CCDData mask/uncertainty attributes are numpy-backed +combiner.py median_combine numpy.asarray BOUNDARY: result stored in astropy CCDData/StdDevUncertainty (numpy-backed) +combiner.py sigma_clipping numpy.asanyarray BOUNDARY: astropy.stats.sigma_clip is numpy-only +combiner.py sum_combine numpy.asarray BOUNDARY: result stored in astropy CCDData/StdDevUncertainty (numpy-backed) +core.py background_deviation_filter numpy.asarray BOUNDARY: scipy.ndimage.generic_filter is numpy-only +core.py block_average numpy.asanyarray BOUNDARY: astropy.nddata.block_reduce is numpy-only +core.py block_reduce numpy.asanyarray BOUNDARY: astropy.nddata.block_reduce is numpy-only +core.py block_replicate numpy.asanyarray BOUNDARY: astropy.nddata.block_replicate is numpy-only +core.py cosmicray_median numpy.asarray BOUNDARY: scipy.ndimage median_filter/maximum_filter are numpy-only +core.py create_deviation numpy.asarray BOUNDARY: astropy CCDData.copy()/StdDevUncertainty are numpy-backed +core.py sigma_func numpy.asanyarray BOUNDARY: astropy.stats.median_absolute_deviation is numpy-only +core.py subtract_overscan numpy.asanyarray BOUNDARY: astropy.modeling fit/evaluation (model= path) is numpy-only diff --git a/ccdproc/tests/pytest_fixtures.py b/ccdproc/tests/pytest_fixtures.py index 23a03670..c3fbae32 100644 --- a/ccdproc/tests/pytest_fixtures.py +++ b/ccdproc/tests/pytest_fixtures.py @@ -50,6 +50,7 @@ def ccd_data( test function, where m is the desired mean. """ # Need the import here to avoid circular import issues + from ..conftest import testing_array_device as xp_device from ..conftest import testing_array_library as xp size = data_size @@ -62,7 +63,7 @@ def ccd_data( data = rng.normal(loc=mean, size=[size, size], scale=scale) fake_meta = {"my_key": 42, "your_key": "not 42"} - ccd = CCDData(xp.asarray(data), unit=u.adu) + ccd = CCDData(xp.asarray(data, device=xp_device), unit=u.adu) ccd.header = fake_meta return ccd diff --git a/ccdproc/tests/test_ccdproc.py b/ccdproc/tests/test_ccdproc.py index bd24a246..6baacc7e 100644 --- a/ccdproc/tests/test_ccdproc.py +++ b/ccdproc/tests/test_ccdproc.py @@ -800,6 +800,11 @@ def tran(arr): # Test block_reduce and block_replicate wrapper @pytest.mark.skipif(not HAS_BLOCK_X_FUNCS, reason="needs astropy >= 1.1.x") +@pytest.mark.backend_xfail( + "array-api-strict", + reason="astropy.nddata.block_reduce is not array-API aware and " + "silently converts back to numpy, which fails on a non-default device", +) def test_block_reduce(): ccd = CCDData( xp.ones((4, 4)), @@ -829,6 +834,11 @@ def test_block_reduce(): @pytest.mark.skipif(not HAS_BLOCK_X_FUNCS, reason="needs astropy >= 1.1.x") +@pytest.mark.backend_xfail( + "array-api-strict", + reason="astropy.nddata.block_reduce is not array-API aware and " + "silently converts back to numpy, which fails on a non-default device", +) def test_block_average(): data = xp.asarray( [ @@ -868,6 +878,11 @@ def test_block_average(): @pytest.mark.skipif(not HAS_BLOCK_X_FUNCS, reason="needs astropy >= 1.1.x") +@pytest.mark.backend_xfail( + "array-api-strict", + reason="astropy.nddata.block_replicate is not array-API aware and " + "silently converts back to numpy, which fails on a non-default device", +) def test_block_replicate(): ccd = CCDData( xp.ones((4, 4)), @@ -916,6 +931,11 @@ def test_create_deviation_does_not_change_input(): assert original.unit == ccd_data.unit +@pytest.mark.backend_xfail( + "array-api-strict", + reason="cosmicray_median uses scipy.ndimage.median_filter, which " + "requires numpy and fails on a non-default device", +) def test_cosmicray_median_does_not_change_input(): ccd_data = ccd_data_func() original = ccd_data.copy() @@ -925,6 +945,11 @@ def test_cosmicray_median_does_not_change_input(): assert original.unit == ccd_data.unit +@pytest.mark.backend_xfail( + "array-api-strict", + reason="cosmicray_lacosmic uses astroscrappy, which requires numpy " + "and fails on a non-default device", +) def test_cosmicray_lacosmic_does_not_change_input(): ccd_data = ccd_data_func() original = ccd_data.copy() @@ -999,6 +1024,11 @@ def wcs_for_testing(shape): return w +@pytest.mark.backend_xfail( + "array-api-strict", + reason="wcs_project uses reproject.reproject_interp, which requires " + "numpy and fails on a non-default device", +) def test_wcs_project_onto_same_wcs(): ccd_data = ccd_data_func() # The trivial case, same WCS, no mask. @@ -1016,6 +1046,11 @@ def test_wcs_project_onto_same_wcs(): assert xp.all(xpx.isclose(ccd_data.data, new_ccd.data, rtol=1e-5)) +@pytest.mark.backend_xfail( + "array-api-strict", + reason="wcs_project uses reproject.reproject_interp, which requires " + "numpy and fails on a non-default device", +) def test_wcs_project_onto_same_wcs_remove_headers(): ccd_data = ccd_data_func() # Remove an example WCS keyword from the header @@ -1031,6 +1066,11 @@ def test_wcs_project_onto_same_wcs_remove_headers(): assert k not in new_ccd.header +@pytest.mark.backend_xfail( + "array-api-strict", + reason="wcs_project uses reproject.reproject_interp, which requires " + "numpy and fails on a non-default device", +) def test_wcs_project_onto_shifted_wcs(): ccd_data = ccd_data_func() # Just make the target WCS the same as the initial with the center @@ -1068,6 +1108,11 @@ def test_wcs_project_onto_shifted_wcs(): # Use an odd number of pixels to make a well-defined center pixel +@pytest.mark.backend_xfail( + "array-api-strict", + reason="wcs_project uses reproject.reproject_interp, which requires " + "numpy and fails on a non-default device", +) def test_wcs_project_onto_scale_wcs(): # Make the target WCS with half the pixel scale and number of pixels # and the values should drop by a factor of 4. diff --git a/ccdproc/tests/test_combiner.py b/ccdproc/tests/test_combiner.py index cf5be461..cb7203b4 100644 --- a/ccdproc/tests/test_combiner.py +++ b/ccdproc/tests/test_combiner.py @@ -345,6 +345,12 @@ def test_combiner_mask_average(): assert not ccd.mask[5, 5] +@pytest.mark.backend_xfail( + "jax", + reason="astropy nddata arithmetic passes a jax array as dtype=, " + "triggering jax's implicit-array-to-dtype DeprecationWarning, which " + "is an error under this test suite's warning filters", +) def test_combiner_with_scaling(): ccd_data = ccd_data_func() # The factors below are not particularly important; just avoid anything @@ -480,6 +486,12 @@ def test_combine_numpyndarray(): assert xp.all(xpx.isclose(avgccd.data, ccd_by_combiner.data)) +@pytest.mark.backend_xfail( + "jax", + reason="astropy nddata arithmetic passes a jax array as dtype=, " + "triggering jax's implicit-array-to-dtype DeprecationWarning, which " + "is an error under this test suite's warning filters", +) def test_combiner_result_dtype(): """Regression test: #391 @@ -742,6 +754,12 @@ def test_combine_result_uncertainty_and_mask(comb_func, mask_point): assert ccd_comb.mask.sum() == mask_point +@pytest.mark.backend_xfail( + "jax", + reason="astropy nddata arithmetic passes a jax array as dtype=, " + "triggering jax's implicit-array-to-dtype DeprecationWarning, which " + "is an error under this test suite's warning filters", +) def test_combine_overwrite_output(tmp_path): """ The combine function should *not* overwrite the result file @@ -1056,6 +1074,12 @@ def create_gen(): assert c._data_arr_mask.shape == (3, 100, 100) +@pytest.mark.backend_xfail( + "jax", + reason="astropy nddata arithmetic passes a jax array as dtype=, " + "triggering jax's implicit-array-to-dtype DeprecationWarning, which " + "is an error under this test suite's warning filters", +) @pytest.mark.parametrize( "comb_func", ["average_combine", "median_combine", "sum_combine"] ) diff --git a/ccdproc/tests/test_cosmicray.py b/ccdproc/tests/test_cosmicray.py index bc77e2ca..8189b6f4 100644 --- a/ccdproc/tests/test_cosmicray.py +++ b/ccdproc/tests/test_cosmicray.py @@ -47,6 +47,11 @@ def add_cosmicrays(data, scale, threshold, ncrays=NCRAYS): data.data = xp.asarray(data_as_np) +@pytest.mark.backend_xfail( + "array-api-strict", + reason="cosmicray_lacosmic uses astroscrappy, which requires numpy " + "and fails on a non-default device", +) def test_cosmicray_lacosmic(): ccd_data = ccd_data_func(data_scale=DATA_SCALE) threshold = 10 @@ -59,6 +64,11 @@ def test_cosmicray_lacosmic(): assert crarr.sum() == NCRAYS +@pytest.mark.backend_xfail( + "array-api-strict", + reason="cosmicray_lacosmic uses astroscrappy, which requires numpy " + "and fails on a non-default device", +) def test_cosmicray_lacosmic_ccddata(): ccd_data = ccd_data_func(data_scale=DATA_SCALE) threshold = 5 @@ -81,6 +91,11 @@ def test_cosmicray_lacosmic_check_data(): cosmicray_lacosmic(10, noise) +@pytest.mark.backend_xfail( + "array-api-strict", + reason="cosmicray_lacosmic uses astroscrappy, which requires numpy " + "and fails on a non-default device", +) @pytest.mark.parametrize("array_input", [True, False]) @pytest.mark.parametrize("gain_correct_data", [True, False]) def test_cosmicray_gain_correct(array_input, gain_correct_data): @@ -122,6 +137,11 @@ def test_cosmicray_gain_correct(array_input, gain_correct_data): assert_allclose(gain_for_test * orig_data, new_data) +@pytest.mark.backend_xfail( + "array-api-strict", + reason="cosmicray_lacosmic uses astroscrappy, which requires numpy " + "and fails on a non-default device", +) def test_cosmicray_lacosmic_accepts_quantity_gain(): ccd_data = ccd_data_func(data_scale=DATA_SCALE) threshold = 5 @@ -135,6 +155,11 @@ def test_cosmicray_lacosmic_accepts_quantity_gain(): _ = cosmicray_lacosmic(ccd_data, gain=gain, gain_apply=True) +@pytest.mark.backend_xfail( + "array-api-strict", + reason="cosmicray_lacosmic uses astroscrappy, which requires numpy " + "and fails on a non-default device", +) def test_cosmicray_lacosmic_accepts_quantity_readnoise(): ccd_data = ccd_data_func(data_scale=DATA_SCALE) threshold = 5 @@ -148,6 +173,11 @@ def test_cosmicray_lacosmic_accepts_quantity_readnoise(): _ = cosmicray_lacosmic(ccd_data, gain=gain, gain_apply=True, readnoise=readnoise) +@pytest.mark.backend_xfail( + "array-api-strict", + reason="cosmicray_lacosmic uses astroscrappy, which requires numpy " + "and fails on a non-default device", +) def test_cosmicray_lacosmic_detects_inconsistent_units(): # This is intended to detect cases like a ccd with units # of adu, a readnoise in electrons and a gain in adu / electron. @@ -168,6 +198,11 @@ def test_cosmicray_lacosmic_detects_inconsistent_units(): assert "Inconsistent units" in str(e.value) +@pytest.mark.backend_xfail( + "array-api-strict", + reason="cosmicray_lacosmic uses astroscrappy, which requires numpy " + "and fails on a non-default device", +) def test_cosmicray_lacosmic_warns_on_ccd_in_electrons(): # Check that an input ccd in electrons raises a warning. ccd_data = ccd_data_func(data_scale=DATA_SCALE) @@ -191,6 +226,11 @@ def test_cosmicray_lacosmic_warns_on_ccd_in_electrons(): # The values for inbkg and invar are DELIBERATELY BAD. They are supposed to be # arrays, so if detect_cosmics is called with these bad values a ValueError # will be raised, which we can check for. +@pytest.mark.backend_xfail( + "array-api-strict", + reason="cosmicray_lacosmic uses astroscrappy, which requires numpy " + "and fails on a non-default device", +) @pytest.mark.parametrize( "new_args", [dict(inbkg=5), dict(invar=5), dict(inbkg=5, invar=5)] ) @@ -214,6 +254,11 @@ def test_cosmicray_median_check_data(): ndata, crarr = cosmicray_median(10, thresh=5, mbox=11, error_image=DATA_SCALE) +@pytest.mark.backend_xfail( + "array-api-strict", + reason="cosmicray_median uses scipy.ndimage.median_filter, which " + "requires numpy and fails on a non-default device", +) def test_cosmicray_median(): ccd_data = ccd_data_func(data_scale=DATA_SCALE) threshold = 5 @@ -226,6 +271,11 @@ def test_cosmicray_median(): assert crarr.sum() == NCRAYS +@pytest.mark.backend_xfail( + "array-api-strict", + reason="cosmicray_median uses scipy.ndimage.median_filter, which " + "requires numpy and fails on a non-default device", +) def test_cosmicray_median_ccddata(): ccd_data = ccd_data_func(data_scale=DATA_SCALE) threshold = 5 @@ -238,6 +288,11 @@ def test_cosmicray_median_ccddata(): assert nccd.mask.sum() == NCRAYS +@pytest.mark.backend_xfail( + "array-api-strict", + reason="cosmicray_median uses scipy.ndimage.median_filter, which " + "requires numpy and fails on a non-default device", +) def test_cosmicray_median_masked(): ccd_data = ccd_data_func(data_scale=DATA_SCALE) threshold = 5 @@ -249,6 +304,11 @@ def test_cosmicray_median_masked(): assert crarr.sum() == NCRAYS +@pytest.mark.backend_xfail( + "array-api-strict", + reason="cosmicray_median uses scipy.ndimage.median_filter, which " + "requires numpy and fails on a non-default device", +) def test_cosmicray_median_background_None(): ccd_data = ccd_data_func(data_scale=DATA_SCALE) threshold = 5 @@ -259,6 +319,11 @@ def test_cosmicray_median_background_None(): assert crarr.sum() == NCRAYS +@pytest.mark.backend_xfail( + "array-api-strict", + reason="cosmicray_median uses scipy.ndimage.median_filter, which " + "requires numpy and fails on a non-default device", +) def test_cosmicray_median_gbox(): ccd_data = ccd_data_func(data_scale=DATA_SCALE) scale = DATA_SCALE # yuck. Maybe use pytest.parametrize? @@ -273,6 +338,11 @@ def test_cosmicray_median_gbox(): assert abs(data.std() - scale) < 0.1 +@pytest.mark.backend_xfail( + "array-api-strict", + reason="cosmicray_median uses scipy.ndimage.median_filter, which " + "requires numpy and fails on a non-default device", +) def test_cosmicray_median_rbox(): ccd_data = ccd_data_func(data_scale=DATA_SCALE) scale = DATA_SCALE # yuck. Maybe use pytest.parametrize? @@ -322,6 +392,11 @@ def test_background_deviation_filter_fail(): # This test can be removed in ccdproc 3.0 when support for old # astroscrappy is removed. +@pytest.mark.backend_xfail( + "array-api-strict", + reason="cosmicray_lacosmic uses astroscrappy, which requires numpy " + "and fails on a non-default device", +) def test_cosmicray_lacosmic_pssl_deprecation_warning(): ccd_data = ccd_data_func(data_scale=DATA_SCALE) with pytest.warns(AstropyDeprecationWarning): @@ -339,6 +414,11 @@ def test_cosmicray_lacosmic_pssl_and_inbkg_fails(): assert "pssl and inbkg" in str(err) +@pytest.mark.backend_xfail( + "array-api-strict", + reason="cosmicray_lacosmic uses astroscrappy, which requires numpy " + "and fails on a non-default device", +) def test_cosmicray_lacosmic_pssl_does_not_fail(): # This test is a copy/paste of test_cosmicray_lacosmic_ccddata # except with pssl=0.0001 as an argument. Subtracting nearly zero from diff --git a/ccdproc/tests/test_image_collection.py b/ccdproc/tests/test_image_collection.py index 22ab819d..ca8f2878 100644 --- a/ccdproc/tests/test_image_collection.py +++ b/ccdproc/tests/test_image_collection.py @@ -79,7 +79,6 @@ def test_repr_files(self, triage_setup): assert repr(ic) == ref def test_repr_ext(self, triage_setup): - hdul = fits.HDUList( [fits.PrimaryHDU(np.ones((10, 10))), fits.ImageHDU(np.ones((10, 10)))] ) @@ -384,6 +383,13 @@ def test_generator_data(self, triage_setup): for img in collection.data(): assert isinstance(img, np.ndarray) + @pytest.mark.backend_xfail( + "jax", + reason="on Linux CI with the jax backend, ccds() does not raise " + "ValueError for unit-less files; the same test passes on macOS with " + "identical astropy/jax versions. Platform-dependent, root cause not " + "yet understood.", + ) def test_generator_ccds_without_unit(self, triage_setup): collection = ImageFileCollection( location=triage_setup.test_dir, keywords=["imagetyp"] @@ -691,7 +697,7 @@ def test_header_with_long_history_roundtrips_to_disk(self, triage_setup): @pytest.mark.skipif( "os.environ.get('APPVEYOR') or os.sys.platform == 'win32'", - reason="fails on Windows because file " "overwriting fails", + reason="fails on Windows because file overwriting fails", ) def test_refresh_method_sees_added_keywords(self, triage_setup): ic = ImageFileCollection(triage_setup.test_dir, keywords="*") diff --git a/docs/array_api.rst b/docs/array_api.rst index 8e54fd4e..1fcceeca 100644 --- a/docs/array_api.rst +++ b/docs/array_api.rst @@ -31,6 +31,121 @@ Though the with `sparse`_. A `pull request `_ to add support for `sparse`_ would be a welcome contribution to the project. +For development purposes, `ccdproc`_'s test suite can also be run against +`array-api-strict`_, a thin wrapper around `numpy`_ that strictly enforces +the array API, rejecting any usage outside the standard, and simulates +multiple devices. This +is set with the environment variable ``CCDPROC_ARRAY_LIBRARY=array-api-strict`` +(``array_api_strict`` is also accepted). By default the test suite creates +arrays on one of `array-api-strict`_'s non-default devices, which causes +``numpy.asarray`` to raise an error, the same way it would on an array still +resident on a `CuPy`_ GPU device. This makes `array-api-strict`_ a convenient +CPU-only proxy for catching places where `ccdproc`_ silently (and +incorrectly) converts a non-numpy array back to numpy. The device used can be +overridden with the ``CCDPROC_ARRAY_DEVICE`` environment variable (its value +is passed to ``array_api_strict.Device``); set it to ``default`` to use the +library's normal CPU device instead. + +A few more developer tools help triage failures on non-numpy backends: + ++ Setting ``CCDPROC_TRIAGE_ESCAPES=1`` prints a summary at the end of the + test session that groups failures by "escape site" -- the innermost frame + inside `ccdproc`_ (but outside its test suite) in each failure's + traceback -- so a large batch of backend failures collapses to a short + list of root-cause call sites. ++ Setting ``CCDPROC_LOG_ARRAY_ESCAPES=1`` logs a warning whenever a + non-numpy array-API array is passed to ``numpy.asarray``, + ``numpy.asanyarray`` or ``numpy.ma.asanyarray``. This catches backends + like `dask`_ and `jax`_ where the conversion succeeds silently and the + test passes anyway. Because the messages go through Python's ``logging`` + and pytest only shows captured logs for *failing* tests, run with + ``-o log_cli=true`` to see escapes from passing tests (the log level is + already configured in ``pyproject.toml``). ++ The ``backend_xfail(*backends, reason=...)`` marker marks a test as an + expected (non-strict) failure only when ``CCDPROC_ARRAY_LIBRARY`` matches + one of the named backends. The ``backend_skip(*backends, reason=...)`` + marker skips a test entirely for the named backends. Because these + xfails are *non-strict* (unlike the suite-wide ``xfail_strict = true`` + default), a backend bug that later gets fixed becomes a silent XPASS + rather than a failure, and the stale marker lingers forever -- check for + XPASSes occasionally (run the backend suite with ``-rX``) and prune the + markers that no longer fail. Before deleting one, confirm the XPASS in + the CI logs rather than only on your own machine: backend behavior can be + platform-dependent (see + `issue #943 `_, where a + jax-marked test passes on macOS but still fails on Linux CI). ++ Setting ``CCDPROC_ENFORCE_ESCAPE_BASELINE=1`` fails the test session if a + new library escape site appears that is not in the checked-in baseline, + and setting ``CCDPROC_WRITE_ESCAPE_BASELINE=1`` regenerates that baseline. + Both are described in "The escape-baseline ratchet" below. + +The escape-baseline ratchet +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The file ``ccdproc/tests/array_escape_baseline.txt`` is a checked-in list of +the known places in the `ccdproc`_ library where a non-numpy array is still +silently converted to numpy, as reported by the escape logger. Each entry is +one line of `` `` followed by an optional +free-text reason (e.g. ``TODO`` for sites still to migrate, or ``BOUNDARY`` +for calls into numpy-only dependencies such as scipy that will never leave). + +Setting ``CCDPROC_ENFORCE_ESCAPE_BASELINE=1`` turns that list into a +ratchet: at the end of the test session, any library escape site *not* in +the baseline fails the session, so new numpy escapes cannot creep in while +the existing ones are migrated. Enforcement checks the sites recorded by the +escape logger, so it is only meaningful with ``CCDPROC_LOG_ARRAY_ESCAPES=1`` +and a non-numpy ``CCDPROC_ARRAY_LIBRARY`` (on numpy there are no foreign +arrays to escape); the session errors out if these are not set. The +``enforce`` tox factor sets the required test-tooling variables and composes +with a backend factor, e.g.:: + + tox -e py312-alldeps-dask-enforce + +To regenerate the baseline, run the *full* test suite from the source tree +(not under tox, which runs the tests against an installed copy of the +package from a temporary directory) with all three of +``CCDPROC_WRITE_ESCAPE_BASELINE=1``, ``CCDPROC_LOG_ARRAY_ESCAPES=1`` and a +non-numpy backend set -- write mode errors out if any of them is missing:: + + CCDPROC_ARRAY_LIBRARY=dask CCDPROC_LOG_ARRAY_ESCAPES=1 \ + CCDPROC_WRITE_ESCAPE_BASELINE=1 pytest + +The file is rewritten from the escapes actually observed during the run, so +a partial run (a subset of the tests) silently drops the entries for code +that was not exercised -- always regenerate over the whole suite. +Hand-written reasons on entries that are still observed are preserved. + +If the enforce CI job (e.g. ``py312-alldeps-dask-enforce``) fails on your +pull request, look for the "NEW escapes" list in the ``ccdproc array-API +escape baseline`` section of the pytest terminal summary. For each new site, +either fix the call site so the data stays in the array-API world +(preferred), or -- if the escape is a deliberate boundary with a numpy-only +dependency -- add the new `` `` line, with a +reason, to ``ccdproc/tests/array_escape_baseline.txt``. The same summary +also lists baseline entries that were not hit this run; delete them if your +change removed the escape. + +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. + What limitations should I be aware of? ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -104,6 +219,7 @@ There are two ways to use the array API in `ccdproc`_: .. _array API: https://data-apis.org/array-api/latest/index.html .. _array-api-compat: https://data-apis.org/array-api-compat +.. _array-api-strict: https://data-apis.org/array-api-strict/ .. _bottleneck: https://bottleneck.readthedocs.io/en/latest/ .. _ccdproc: https://ccdproc.readthedocs.io/en/latest/ .. _cupy: https://docs.cupy.dev/en/stable/ diff --git a/pyproject.toml b/pyproject.toml index eee6170a..7f5a5ba4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -55,6 +55,14 @@ include = [ "/licenses", ] +[tool.hatch.build.targets.wheel] +# The array-API escape baseline is read at test time by ccdproc/tests, so it +# must ship in the wheel (not just the sdist) for the CCDPROC_ENFORCE_ESCAPE_ +# BASELINE ratchet to find it when tox installs the built package. +artifacts = [ + "ccdproc/tests/array_escape_baseline.txt", +] + [tool.black] line-length = 88 target-version = ['py311', 'py312', 'py313'] @@ -157,4 +165,6 @@ markers = [ "data_size(N): set dimension of square data array for ccd_data fixture", "data_scale(s): set the scale of the normal distribution used to generate data", "data_mean(m): set the center of the normal distribution used to generate data", + "backend_xfail(*backends, reason=...): mark test as an expected (non-strict) failure when the active CCDPROC_ARRAY_LIBRARY matches one of the named backends", + "backend_skip(*backends, reason=...): skip test when the active CCDPROC_ARRAY_LIBRARY matches one of the named backends", ] diff --git a/tox.ini b/tox.ini index 4d2e63c4..943bcfe0 100644 --- a/tox.ini +++ b/tox.ini @@ -10,12 +10,39 @@ setenv = jax: CCDPROC_ARRAY_LIBRARY = jax jax: JAX_ENABLE_X64 = True dask: CCDPROC_ARRAY_LIBRARY = dask + strict: CCDPROC_ARRAY_LIBRARY = array-api-strict + strict: CCDPROC_TRIAGE_ESCAPES = 1 + # Local-only factor for turning on escape logging with other backends, + # e.g. py312-test-dask-escapes. Not used in CI. Enables both the + # np.asarray escape logger (catches silent conversions even in passing + # tests) and the failure-triage summary. + escapes: CCDPROC_LOG_ARRAY_ESCAPES = 1 + escapes: CCDPROC_TRIAGE_ESCAPES = 1 + # Live logging so escapes logged during *passing* tests are shown; + # pytest only prints captured logs for failing tests otherwise. + escapes: PYTEST_ADDOPTS = -o log_cli=true + # CI gate: compare the live-logged library escapes against the checked-in + # baseline (ccdproc/tests/array_escape_baseline.txt) and fail the run if a + # *new* escape appears. Enforcement needs the escape logger, so this + # factor deliberately sets both variables (pytest_sessionstart in + # ccdproc/tests/_escape_triage.py rejects enforce mode without the + # logger); compose with a backend factor, e.g. + # py312-alldeps-dask-enforce. Refresh the baseline locally (in the source + # tree, not under tox) with CCDPROC_WRITE_ESCAPE_BASELINE=1. + enforce: CCDPROC_LOG_ARRAY_ESCAPES = 1 + enforce: CCDPROC_ENFORCE_ESCAPE_BASELINE = 1 devdeps: PIP_EXTRA_INDEX_URL = https://pypi.anaconda.org/astropy/simple extras = test # Run the tests in a temporary directory to make sure that we don't # import this package from the source tree +# +# The CI strict/enforce envs (py313-strict, py312-alldeps-dask-enforce) +# deliberately omit the "test" factor so they run from the source tree +# instead of .tmp/{envname}. Do not rename them to py31x-test-...: that +# silently changes which copy of ccdproc is imported and where the +# __file__-anchored baseline path in ccdproc/tests/_escape_triage.py points. changedir = test: .tmp/{envname} @@ -30,6 +57,9 @@ description = numpy210: with numpy 2.1.* bottleneck: with bottleneck jax: with JAX as the array library + strict: with array-api-strict as the array library + escapes: with escape-triage logging enabled (local use only) + enforce: failing the run if a new array-API escape appears (baseline ratchet) # The following provides some specific pinnings for key packages @@ -55,6 +85,11 @@ deps = oldestdeps: reproject==0.9.1 dask: dask + # The strict env intentionally installs only the test extras plus + # array-api-strict (no alldeps): numpy-boundary paths through + # scipy/reproject/astroscrappy are out of scope there, so do not "fix" + # this by adding optional dependencies. + strict: array-api-strict>=2.0 commands = pip freeze