From 94ca9ec47d0d780a844d2274e888f4c8b2b0a5b1 Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Mon, 6 Jul 2026 21:19:51 -0500 Subject: [PATCH 01/19] Add array-api-strict test backend and array-escape triage tooling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire array-api-strict (on a non-default device) into the CCDPROC_ARRAY_LIBRARY test-backend mechanism as a CPU-only proxy for CuPy's implicit device-to-host conversion errors, with a py313-strict tox environment and CI job (#937). Add triage tooling for array-library escapes (#939): - an escape-site dedupe report (CCDPROC_TRIAGE_ESCAPES=1) that groups test failures by the innermost non-test ccdproc frame - backend_xfail/backend_skip markers applied per array backend - an escape logger (CCDPROC_LOG_ARRAY_ESCAPES=1) that warns whenever a non-numpy array-API array is passed to a numpy coercion function Mark tests of documented limitations (astroscrappy, scipy.ndimage, reproject, block_reduce/block_replicate) as backend_xfail for array-api-strict. Closes #937 Closes #939 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Qri8rDgkWjCT7Yb7QKJrhf --- .github/workflows/ci_tests.yml | 5 + CHANGES.rst | 17 +++ ccdproc/conftest.py | 206 +++++++++++++++++++++++++++++++ ccdproc/tests/_escape_triage.py | 131 ++++++++++++++++++++ ccdproc/tests/pytest_fixtures.py | 3 +- ccdproc/tests/test_ccdproc.py | 45 +++++++ ccdproc/tests/test_cosmicray.py | 80 ++++++++++++ docs/array_api.rst | 15 +++ pyproject.toml | 2 + tox.ini | 3 + 10 files changed, 506 insertions(+), 1 deletion(-) create mode 100644 ccdproc/tests/_escape_triage.py diff --git a/.github/workflows/ci_tests.yml b/.github/workflows/ci_tests.yml index 3667baa4..c3a7babc 100644 --- a/.github/workflows/ci_tests.yml +++ b/.github/workflows/ci_tests.yml @@ -47,6 +47,11 @@ jobs: python: '3.13' tox_env: 'py313-jax' + - name: 'ubuntu-py313-strict' + os: ubuntu-latest + python: '3.13' + tox_env: 'py313-strict' + # Move bottleneck test a test without coverage - name: 'ubuntu-py312-bottleneck' os: ubuntu-latest diff --git a/CHANGES.rst b/CHANGES.rst index 3b571709..cef2d655 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,3 +1,20 @@ +Unreleased +---------- + +New Features +^^^^^^^^^^^^ + +- Added support for testing against ``array-api-strict`` on a non-default + device, which serves as a CPU-only proxy for CuPy-style GPU device + behavior. Set ``CCDPROC_ARRAY_LIBRARY=array-api-strict`` (and optionally + ``CCDPROC_ARRAY_DEVICE``) to use it. [#937] +- Added test-infrastructure tooling for triaging array-API backend + failures: ``@pytest.mark.backend_xfail``/``@pytest.mark.backend_skip`` + markers for per-backend expected failures, an escape-site dedupe report + enabled with ``CCDPROC_TRIAGE_ESCAPES=1``, and an array-escape logger + enabled with ``CCDPROC_LOG_ARRAY_ESCAPES=1`` that flags silent conversion + of non-numpy arrays back to numpy. [#939] + 2.5.1 (2025-07-05) ------------------ diff --git a/ccdproc/conftest.py b/ccdproc/conftest.py index 7806f8d5..831bb93e 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.numpy +import numpy as np +import pytest try: # When the pytest_astropy_header package is installed @@ -19,6 +24,10 @@ def pytest_configure(config): TESTED_VERSIONS = {} +from .tests._escape_triage import ( + pytest_runtest_makereport, # noqa: F401 pytest hook, used via attribute lookup + pytest_terminal_summary, # noqa: F401 pytest hook, used via attribute lookup +) from .tests.pytest_fixtures import ( triage_setup, # noqa: F401 this is used in tests ) @@ -41,6 +50,14 @@ 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). It can be +# overridden with the CCDPROC_ARRAY_DEVICE environment variable, whose value +# is passed to the array library's Device constructor. "default" (or leaving +# it unset) means the library's usual default device. +testing_array_device = None + match array_library: case "numpy": import array_api_compat.numpy as testing_array_library # noqa: F401 @@ -60,8 +77,197 @@ 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", "default").lower() + if device_name != "default": + testing_array_device = testing_array_library.Device(device_name) + else: + testing_array_device = testing_array_library.Device("device1") + case _: raise ValueError( f"Unsupported array library: {array_library}. " "Supported libraries are listed at https://ccdproc.readthedocs.io/en/latest/array_api.html." ) + + +def _env_truthy(value): + return str(value).strip().lower() in {"1", "true", "yes", "on"} + + +# --------------------------------------------------------------------------- +# 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): + 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)) + 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 that reports its own `__array_namespace__` and that namespace is +# not numpy. +# +# 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 _is_foreign_array(obj): + if isinstance(obj, np.ndarray): + return False + + get_namespace = getattr(obj, "__array_namespace__", None) + if get_namespace is None: + return False + + try: + namespace = get_namespace() + except Exception: + return False + + return namespace not in _NUMPY_LIKE_NAMESPACES + + +def _describe_escape_site(): + """ + Return a short "file:line function" string for the innermost stack + frame that is inside ccdproc but not inside ccdproc's own test suite, + for use in escape log messages. + """ + from .tests._escape_triage import locate_escape_site + + # Drop this function's own frame before searching. + frames = traceback.extract_stack()[:-1] + site = locate_escape_site(frames) + if site is None: + return "" + return f"{site.filename}:{site.lineno} {site.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): + def wrapper(*args, **kwargs): + if not guard.active and args: + guard.active = True + try: + obj = args[0] + if _is_foreign_array(obj): + site = _describe_escape_site() + _escape_logger.warning( + "array-API escape: %s() called on a %r array " + "(namespace=%r) at %s", + funcname, + type(obj), + obj.__array_namespace__(), + site, + ) + 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..838ffa94 --- /dev/null +++ b/ccdproc/tests/_escape_triage.py @@ -0,0 +1,131 @@ +# 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. + +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", "")) + +#: 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) + + +def _is_ccdproc_frame(filename): + return f"{os.sep}ccdproc{os.sep}" in filename + + +def _is_ccdproc_test_frame(filename): + return f"{os.sep}ccdproc{os.sep}tests{os.sep}" in filename + + +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(hookwrapper=True) +def pytest_runtest_makereport(item, call): + outcome = yield + + if not TRIAGE_ACTIVE: + return + + report = outcome.get_result() + if report.when != "call" or not report.failed: + return + + excinfo = call.excinfo + if excinfo is None: + return + + site = locate_escape_site(traceback.extract_tb(excinfo.tb)) + if site is None: + return + + key = (site.filename, site.lineno, site.name) + _ESCAPE_SITES[key].append(item.nodeid) + + +def pytest_terminal_summary(terminalreporter): + 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" + ) 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_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/docs/array_api.rst b/docs/array_api.rst index 8e54fd4e..e5960494 100644 --- a/docs/array_api.rst +++ b/docs/array_api.rst @@ -31,6 +31,20 @@ 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 reference implementation of the array API that +performs no computation itself but strictly validates array API usage. 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. + What limitations should I be aware of? ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -104,6 +118,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..edefd767 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -157,4 +157,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..3eacd7f4 100644 --- a/tox.ini +++ b/tox.ini @@ -10,6 +10,7 @@ setenv = jax: CCDPROC_ARRAY_LIBRARY = jax jax: JAX_ENABLE_X64 = True dask: CCDPROC_ARRAY_LIBRARY = dask + strict: CCDPROC_ARRAY_LIBRARY = array-api-strict devdeps: PIP_EXTRA_INDEX_URL = https://pypi.anaconda.org/astropy/simple extras = test @@ -30,6 +31,7 @@ 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 # The following provides some specific pinnings for key packages @@ -55,6 +57,7 @@ deps = oldestdeps: reproject==0.9.1 dask: dask + strict: array-api-strict commands = pip freeze From 585418ac806d8fd9e2e32638318cfd547912304c Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Mon, 6 Jul 2026 21:36:35 -0500 Subject: [PATCH 02/19] Do not cancel remaining CI jobs when one fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The py313-strict job fails by design until the remaining array-API bugs are fixed; with the default fail-fast it would cancel the whole matrix. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Qri8rDgkWjCT7Yb7QKJrhf --- .github/workflows/ci_tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci_tests.yml b/.github/workflows/ci_tests.yml index c3a7babc..4f60f987 100644 --- a/.github/workflows/ci_tests.yml +++ b/.github/workflows/ci_tests.yml @@ -25,6 +25,9 @@ jobs: runs-on: ${{ matrix.os }} if: "!(contains(github.event.head_commit.message, '[skip ci]') || contains(github.event.head_commit.message, '[ci skip]'))" strategy: + # The strict array-API job is expected to fail until the remaining + # array-API bugs are fixed; don't let its failure cancel the other jobs. + fail-fast: false matrix: include: - name: 'ubuntu-py311-oldestdeps' From b51f90397f06ba97c5a1b91ec837d15a61c4e005 Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Mon, 6 Jul 2026 21:40:07 -0500 Subject: [PATCH 03/19] Use continue-on-error for the strict array-API job instead of fail-fast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The py313-strict job is expected to fail until the remaining array-API bugs are fixed; continue-on-error keeps its failure from failing the workflow or cancelling the other matrix jobs, while restoring fail-fast for real failures elsewhere. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Qri8rDgkWjCT7Yb7QKJrhf --- .github/workflows/ci_tests.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci_tests.yml b/.github/workflows/ci_tests.yml index 4f60f987..f528d2b1 100644 --- a/.github/workflows/ci_tests.yml +++ b/.github/workflows/ci_tests.yml @@ -24,10 +24,11 @@ jobs: name: ${{ matrix.name }} runs-on: ${{ matrix.os }} if: "!(contains(github.event.head_commit.message, '[skip ci]') || contains(github.event.head_commit.message, '[ci skip]'))" + # Matrix entries with continue-on-error (currently the strict array-API + # job) are expected to fail until the remaining array-API bugs are fixed; + # their failure neither fails the workflow nor cancels the other jobs. + continue-on-error: ${{ matrix.continue-on-error || false }} strategy: - # The strict array-API job is expected to fail until the remaining - # array-API bugs are fixed; don't let its failure cancel the other jobs. - fail-fast: false matrix: include: - name: 'ubuntu-py311-oldestdeps' @@ -54,6 +55,7 @@ jobs: os: ubuntu-latest python: '3.13' tox_env: 'py313-strict' + continue-on-error: true # Move bottleneck test a test without coverage - name: 'ubuntu-py312-bottleneck' From 8eddeb8896084e41374942e716bf3de7a0cc23f2 Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Mon, 6 Jul 2026 21:45:09 -0500 Subject: [PATCH 04/19] Move the strict array-API job into its own CI matrix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The py313-strict job is expected to fail until the remaining array-API bugs are fixed. In its own matrix its failure stays visible but cannot fail-fast-cancel the main test matrix. Reverts the earlier continue-on-error approach, which hid the failure entirely. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Qri8rDgkWjCT7Yb7QKJrhf --- .github/workflows/ci_tests.yml | 46 ++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci_tests.yml b/.github/workflows/ci_tests.yml index f528d2b1..ff002803 100644 --- a/.github/workflows/ci_tests.yml +++ b/.github/workflows/ci_tests.yml @@ -24,10 +24,6 @@ jobs: name: ${{ matrix.name }} runs-on: ${{ matrix.os }} if: "!(contains(github.event.head_commit.message, '[skip ci]') || contains(github.event.head_commit.message, '[ci skip]'))" - # Matrix entries with continue-on-error (currently the strict array-API - # job) are expected to fail until the remaining array-API bugs are fixed; - # their failure neither fails the workflow nor cancels the other jobs. - continue-on-error: ${{ matrix.continue-on-error || false }} strategy: matrix: include: @@ -51,12 +47,6 @@ jobs: python: '3.13' tox_env: 'py313-jax' - - name: 'ubuntu-py313-strict' - os: ubuntu-latest - python: '3.13' - tox_env: 'py313-strict' - continue-on-error: true - # Move bottleneck test a test without coverage - name: 'ubuntu-py312-bottleneck' os: ubuntu-latest @@ -118,3 +108,39 @@ 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. + 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 + run: | + tox -e ${{ matrix.tox_env }} -- ${{ matrix.toxposargs }} From cc3238029e8161321c8e5a4bc6c9d92741b7fd24 Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Mon, 6 Jul 2026 22:13:53 -0500 Subject: [PATCH 05/19] Address review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix CCDPROC_ARRAY_DEVICE=default to select the library CPU device as documented; unset still selects device1 - Reword the device comment in conftest to match the actual behavior - Require array-api-strict>=2.0 in the strict tox factor - Shorten the changelog entries - Add docstrings to _is_foreign_array and the escape-triage pytest hooks - Deduplicate _env_truthy (now imported from _escape_triage) 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Qri8rDgkWjCT7Yb7QKJrhf --- CHANGES.rst | 16 ++++++--------- ccdproc/conftest.py | 35 ++++++++++++++++++++++----------- ccdproc/tests/_escape_triage.py | 16 +++++++++++++++ tox.ini | 2 +- 4 files changed, 46 insertions(+), 23 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index cef2d655..833f7a36 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -4,16 +4,12 @@ Unreleased New Features ^^^^^^^^^^^^ -- Added support for testing against ``array-api-strict`` on a non-default - device, which serves as a CPU-only proxy for CuPy-style GPU device - behavior. Set ``CCDPROC_ARRAY_LIBRARY=array-api-strict`` (and optionally - ``CCDPROC_ARRAY_DEVICE``) to use it. [#937] -- Added test-infrastructure tooling for triaging array-API backend - failures: ``@pytest.mark.backend_xfail``/``@pytest.mark.backend_skip`` - markers for per-backend expected failures, an escape-site dedupe report - enabled with ``CCDPROC_TRIAGE_ESCAPES=1``, and an array-escape logger - enabled with ``CCDPROC_LOG_ARRAY_ESCAPES=1`` that flags silent conversion - of non-numpy arrays back to numpy. [#939] +- Add ``array-api-strict`` (on a non-default device) as a test backend, a + CPU-only proxy for CuPy device behavior. [#937] +- 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``). [#939] 2.5.1 (2025-07-05) ------------------ diff --git a/ccdproc/conftest.py b/ccdproc/conftest.py index 831bb93e..30130b58 100644 --- a/ccdproc/conftest.py +++ b/ccdproc/conftest.py @@ -25,6 +25,7 @@ def pytest_configure(config): from .tests._escape_triage import ( + _env_truthy, pytest_runtest_makereport, # noqa: F401 pytest hook, used via attribute lookup pytest_terminal_summary, # noqa: F401 pytest hook, used via attribute lookup ) @@ -52,10 +53,12 @@ def pytest_configure(config): # 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). It can be -# overridden with the CCDPROC_ARRAY_DEVICE environment variable, whose value -# is passed to the array library's Device constructor. "default" (or leaving -# it unset) means the library's usual default device. +# 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: @@ -89,11 +92,12 @@ def pytest_configure(config): # 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", "default").lower() - if device_name != "default": - testing_array_device = testing_array_library.Device(device_name) + 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("device1") + testing_array_device = testing_array_library.Device(device_name) case _: raise ValueError( @@ -102,10 +106,6 @@ def pytest_configure(config): ) -def _env_truthy(value): - return str(value).strip().lower() in {"1", "true", "yes", "on"} - - # --------------------------------------------------------------------------- # Per-backend xfail/skip markers # @@ -172,6 +172,17 @@ def pytest_collection_modifyitems(items): def _is_foreign_array(obj): + """ + Return True if obj is an array from a non-numpy array-API library. + + numpy arrays (including np.ma masked arrays) are never foreign. Anything + else that implements the array-API protocol marker __array_namespace__() + is foreign unless its namespace 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 objects whose __array_namespace__() raises; those are + treated as not foreign so the logger never breaks the call it wraps. + """ if isinstance(obj, np.ndarray): return False diff --git a/ccdproc/tests/_escape_triage.py b/ccdproc/tests/_escape_triage.py index 838ffa94..4419fec9 100644 --- a/ccdproc/tests/_escape_triage.py +++ b/ccdproc/tests/_escape_triage.py @@ -85,6 +85,15 @@ def locate_escape_site(frames): @pytest.hookimpl(hookwrapper=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. 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. + """ outcome = yield if not TRIAGE_ACTIVE: @@ -107,6 +116,13 @@ def pytest_runtest_makereport(item, call): def pytest_terminal_summary(terminalreporter): + """ + Print the escape-site summary at the end of the test session. + + 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 diff --git a/tox.ini b/tox.ini index 3eacd7f4..ad357bf8 100644 --- a/tox.ini +++ b/tox.ini @@ -57,7 +57,7 @@ deps = oldestdeps: reproject==0.9.1 dask: dask - strict: array-api-strict + strict: array-api-strict>=2.0 commands = pip freeze From b5bf86ece4eaae55f84c21c052895525f0b00675 Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Mon, 6 Jul 2026 22:19:59 -0500 Subject: [PATCH 06/19] Enable the escape-triage summary in the strict tox environment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The py313-strict CI job existed to surface the escape-site report, but the strict tox factor never set CCDPROC_TRIAGE_ESCAPES, so the summary was not generated. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Qri8rDgkWjCT7Yb7QKJrhf --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index ad357bf8..f919880b 100644 --- a/tox.ini +++ b/tox.ini @@ -11,6 +11,7 @@ setenv = jax: JAX_ENABLE_X64 = True dask: CCDPROC_ARRAY_LIBRARY = dask strict: CCDPROC_ARRAY_LIBRARY = array-api-strict + strict: CCDPROC_TRIAGE_ESCAPES = 1 devdeps: PIP_EXTRA_INDEX_URL = https://pypi.anaconda.org/astropy/simple extras = test From fdf651e61f261d3ffafca197cd8a1ec84574551b Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Mon, 6 Jul 2026 22:22:22 -0500 Subject: [PATCH 07/19] Anchor escape-triage frame detection on the package directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On CI the repository checkout is itself named ccdproc, so testing for /ccdproc/ anywhere in a frame's path classified every frame -- including tox's site-packages under the workspace -- as a ccdproc frame, and the triage report pointed at array_api_strict internals instead of the ccdproc call site. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Qri8rDgkWjCT7Yb7QKJrhf --- ccdproc/tests/_escape_triage.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/ccdproc/tests/_escape_triage.py b/ccdproc/tests/_escape_triage.py index 4419fec9..470d2ed6 100644 --- a/ccdproc/tests/_escape_triage.py +++ b/ccdproc/tests/_escape_triage.py @@ -39,12 +39,20 @@ def _env_truthy(value): _ESCAPE_SITES = defaultdict(list) +# 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 + + def _is_ccdproc_frame(filename): - return f"{os.sep}ccdproc{os.sep}" in filename + return os.path.abspath(filename).startswith(_PACKAGE_ROOT) def _is_ccdproc_test_frame(filename): - return f"{os.sep}ccdproc{os.sep}tests{os.sep}" in filename + return os.path.abspath(filename).startswith(_TESTS_ROOT) def locate_escape_site(frames): From a9313a6fdff21081633c5636d869a4a6ce46d29f Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Tue, 7 Jul 2026 21:06:41 -0500 Subject: [PATCH 08/19] Fix escape-logger bugs and apply agreed review changes - Classify ccdproc/conftest.py as test infrastructure in the escape-site frame classifiers, so the escape logger's own monkeypatch wrapper is never reported as the escape site and no longer corrupts the triage summary when both flags are enabled. - Detect foreign arrays via array_api_compat.array_namespace() instead of the raw __array_namespace__ dunder, so dask arrays (which lack the dunder) are detected; return the namespace from the helper so the log message no longer re-invokes third-party code unguarded. - Convert the triage hook to a new-style pluggy wrapper. - Document the triage/escape-logger flags and backend markers in docs/array_api.rst, including the -o log_cli=true requirement for seeing escapes from passing tests, and correct the description of array-api-strict. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Qri8rDgkWjCT7Yb7QKJrhf --- ccdproc/conftest.py | 51 +++++++++++++++++---------------- ccdproc/tests/_escape_triage.py | 51 ++++++++++++++++++--------------- docs/array_api.rst | 25 ++++++++++++++-- 3 files changed, 77 insertions(+), 50 deletions(-) diff --git a/ccdproc/conftest.py b/ccdproc/conftest.py index 30130b58..ce0ce0b8 100644 --- a/ccdproc/conftest.py +++ b/ccdproc/conftest.py @@ -7,7 +7,7 @@ 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 @@ -153,8 +153,8 @@ def pytest_collection_modifyitems(items): # 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 that reports its own `__array_namespace__` and that namespace is -# not numpy. +# an array whose array-API namespace (as resolved by +# array_api_compat.array_namespace) is not numpy. # # Activated by setting the environment variable CCDPROC_LOG_ARRAY_ESCAPES to # a truthy value. @@ -171,31 +171,31 @@ def pytest_collection_modifyitems(items): _NUMPY_LIKE_NAMESPACES = {np, array_api_compat.numpy} -def _is_foreign_array(obj): +def _foreign_namespace(obj): """ - Return True if obj is an array from a non-numpy array-API library. - - numpy arrays (including np.ma masked arrays) are never foreign. Anything - else that implements the array-API protocol marker __array_namespace__() - is foreign unless its namespace 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 objects whose __array_namespace__() raises; those are - treated as not foreign so the logger never breaks the call it wraps. + 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 False - - get_namespace = getattr(obj, "__array_namespace__", None) - if get_namespace is None: - return False - + return None try: - namespace = get_namespace() + namespace = array_api_compat.array_namespace(obj) + if namespace in _NUMPY_LIKE_NAMESPACES: + return None except Exception: - return False - - return namespace not in _NUMPY_LIKE_NAMESPACES + return None + return namespace def _describe_escape_site(): @@ -227,14 +227,15 @@ def wrapper(*args, **kwargs): guard.active = True try: obj = args[0] - if _is_foreign_array(obj): + namespace = _foreign_namespace(obj) + if namespace is not None: site = _describe_escape_site() _escape_logger.warning( "array-API escape: %s() called on a %r array " "(namespace=%r) at %s", funcname, type(obj), - obj.__array_namespace__(), + namespace, site, ) finally: diff --git a/ccdproc/tests/_escape_triage.py b/ccdproc/tests/_escape_triage.py index 470d2ed6..e177bc6d 100644 --- a/ccdproc/tests/_escape_triage.py +++ b/ccdproc/tests/_escape_triage.py @@ -45,6 +45,7 @@ def _env_truthy(value): # 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): @@ -52,7 +53,11 @@ def _is_ccdproc_frame(filename): def _is_ccdproc_test_frame(filename): - return os.path.abspath(filename).startswith(_TESTS_ROOT) + # 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 def locate_escape_site(frames): @@ -91,36 +96,36 @@ def locate_escape_site(frames): return frames[-1] -@pytest.hookimpl(hookwrapper=True) +@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. The failure's traceback is reduced to its most informative - frame by locate_escape_site() and the test id is filed under that + 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. """ - outcome = yield - - if not TRIAGE_ACTIVE: - return - - report = outcome.get_result() - if report.when != "call" or not report.failed: - return - - excinfo = call.excinfo - if excinfo is None: - return - - site = locate_escape_site(traceback.extract_tb(excinfo.tb)) - if site is None: - return - - key = (site.filename, site.lineno, site.name) - _ESCAPE_SITES[key].append(item.nodeid) + # 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): diff --git a/docs/array_api.rst b/docs/array_api.rst index e5960494..8203059c 100644 --- a/docs/array_api.rst +++ b/docs/array_api.rst @@ -32,8 +32,9 @@ with `sparse`_. A `pull request `_ to 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 reference implementation of the array API that -performs no computation itself but strictly validates array API usage. This +`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 @@ -45,6 +46,26 @@ 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. + What limitations should I be aware of? ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From cf4f4b7ec5a38cc0d1134cf5a7f19062482499d2 Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Tue, 7 Jul 2026 21:32:34 -0500 Subject: [PATCH 09/19] Mark tests failing on the jax backend as backend_xfail The jax CI job's failure cancels the rest of the test matrix via fail-fast. Mark the seven tests that currently fail there (jax 0.10 / astropy 8) as non-strict expected failures so the matrix completes: - Six combiner tests fail because astropy's nddata arithmetic passes a jax array as dtype=, which raises jax's implicit-array-to-dtype DeprecationWarning as an error under the suite's warning filters. - test_generator_ccds_without_unit does not raise the expected ValueError on the jax backend. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Qri8rDgkWjCT7Yb7QKJrhf --- ccdproc/tests/test_combiner.py | 24 ++++++++++++++++++++++++ ccdproc/tests/test_image_collection.py | 8 ++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) 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_image_collection.py b/ccdproc/tests/test_image_collection.py index 22ab819d..d120f6b7 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,11 @@ def test_generator_data(self, triage_setup): for img in collection.data(): assert isinstance(img, np.ndarray) + @pytest.mark.backend_xfail( + "jax", + reason="ccds() on the jax backend does not raise ValueError when " + "the files have no unit", + ) def test_generator_ccds_without_unit(self, triage_setup): collection = ImageFileCollection( location=triage_setup.test_dir, keywords=["imagetyp"] @@ -691,7 +695,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="*") From 30126fb0a6a08fa652d6a22a5f75d346d9781e60 Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Wed, 8 Jul 2026 09:44:45 -0500 Subject: [PATCH 10/19] Add local-only escapes tox factor for dask and jax runs The escapes factor sets CCDPROC_LOG_ARRAY_ESCAPES (warns on silent conversion of foreign arrays to numpy, even in passing tests) and CCDPROC_TRIAGE_ESCAPES (groups test failures by escape site), and turns on pytest live logging via PYTEST_ADDOPTS so the escape warnings are visible for passing tests. Enables environments like py312-test-dask-escapes and py312-test-jax-escapes for local triage. Not added to CI. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01BMP3e8QVgzf932GvPP2mTJ --- tox.ini | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tox.ini b/tox.ini index f919880b..8fc0855a 100644 --- a/tox.ini +++ b/tox.ini @@ -12,6 +12,15 @@ setenv = 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 devdeps: PIP_EXTRA_INDEX_URL = https://pypi.anaconda.org/astropy/simple extras = test @@ -33,6 +42,7 @@ description = 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) # The following provides some specific pinnings for key packages From eed1fb039a7cf50aa560d3d109ddd4e7132ec661 Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Wed, 8 Jul 2026 19:38:24 -0500 Subject: [PATCH 11/19] Add array-API escape baseline ratchet and dedup escape-log summary Extend the escape tooling so silent numpy coercions on non-numpy backends (dask/jax) can be gated in CI, not just eyeballed in a log. - The live escape logger now tallies every coercion and prints a deduplicated "escape log summary" (counts per file:line function) at the end of the session. The conftest wrapper passes the blamed FrameSummary to record_escape_log() so the summary and the ratchet share one source. - New baseline ratchet: ccdproc/tests/array_escape_baseline.txt lists the known library escape sites keyed on (file, function, coercion) -- no line numbers, to avoid churn. CCDPROC_ENFORCE_ESCAPE_BASELINE=1 fails the session (via pytest_sessionfinish) if a new library escape appears; CCDPROC_WRITE_ESCAPE_BASELINE=1 (re)seeds the file, preserving hand-written reason tags. Test-frame escapes are excluded via _is_library_site. - The 13 current dask sites are all third-party numpy-only boundaries (astropy CCDData/stats/nddata/modeling, scipy.ndimage), tagged BOUNDARY with reasons; no pure-ccdproc coercions remain, so the ratchet is a regression gate. - tox: new `enforce` factor (compose e.g. py312-alldeps-dask-enforce) turns on the logger and enforcement. pyproject ships the baseline in the wheel so tox's installed-package run finds it. CI: new must-pass job ubuntu-py312-dask-escape-baseline runs the gate. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01GHqzFdpg3qqiBb29ETq35D --- .github/workflows/ci_tests.yml | 10 + ccdproc/conftest.py | 28 ++- ccdproc/tests/_escape_triage.py | 258 +++++++++++++++++++++++- ccdproc/tests/array_escape_baseline.txt | 25 +++ pyproject.toml | 8 + tox.ini | 9 + 6 files changed, 327 insertions(+), 11 deletions(-) create mode 100644 ccdproc/tests/array_escape_baseline.txt diff --git a/.github/workflows/ci_tests.yml b/.github/workflows/ci_tests.yml index ff002803..1d1951f8 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' diff --git a/ccdproc/conftest.py b/ccdproc/conftest.py index ce0ce0b8..3ee53907 100644 --- a/ccdproc/conftest.py +++ b/ccdproc/conftest.py @@ -27,7 +27,9 @@ def pytest_configure(config): 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_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 @@ -198,20 +200,24 @@ def _foreign_namespace(obj): return namespace -def _describe_escape_site(): +def _escape_site_frame(): """ - Return a short "file:line function" string for the innermost stack - frame that is inside ccdproc but not inside ccdproc's own test suite, - for use in escape log messages. + 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 - # Drop this function's own frame before searching. - frames = traceback.extract_stack()[:-1] - site = locate_escape_site(frames) - if site is None: + 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"{site.filename}:{site.lineno} {site.name}" + return f"{frame.filename}:{frame.lineno} {frame.name}" class _ReentrancyGuard: @@ -229,7 +235,8 @@ def wrapper(*args, **kwargs): obj = args[0] namespace = _foreign_namespace(obj) if namespace is not None: - site = _describe_escape_site() + 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", @@ -238,6 +245,7 @@ def wrapper(*args, **kwargs): namespace, site, ) + record_escape_log(frame, funcname) finally: guard.active = False return original(*args, **kwargs) diff --git a/ccdproc/tests/_escape_triage.py b/ccdproc/tests/_escape_triage.py index e177bc6d..35524761 100644 --- a/ccdproc/tests/_escape_triage.py +++ b/ccdproc/tests/_escape_triage.py @@ -34,10 +34,43 @@ def _env_truthy(value): 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", "")) + #: 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 @@ -60,6 +93,124 @@ def _is_ccdproc_test_frame(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()) + + +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. + """ + sites = sorted(_observed_library_sites()) + existing = _load_escape_baseline() + 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).", + "#", + "# 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.", + "#", + ] + if not sites: + _write_lines(header + ["# (no library escapes observed this run)"]) + return + w_file = max(len(f) for f, _, _ in sites) + w_func = max(len(fn) for _, fn, _ in sites) + w_co = max(len(c) for _, _, c in sites) + 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 @@ -130,7 +281,23 @@ def pytest_runtest_makereport(item, call): def pytest_terminal_summary(terminalreporter): """ - Print the escape-site summary at the end of the test session. + 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) + + +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 @@ -158,3 +325,92 @@ def pytest_terminal_summary(terminalreporter): 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 on a numpy backend + # (or was never installed). 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( + "No foreign-array escapes observed (numpy backend?); " + "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 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. 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/pyproject.toml b/pyproject.toml index edefd767..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'] diff --git a/tox.ini b/tox.ini index 8fc0855a..be17161f 100644 --- a/tox.ini +++ b/tox.ini @@ -21,6 +21,14 @@ setenv = # 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 turns it on too; compose with a backend factor, e.g. + # py312-test-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 @@ -43,6 +51,7 @@ description = 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 From 74574ccad04911e464be303ad1312043fea3b48d Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Fri, 10 Jul 2026 22:07:37 -0500 Subject: [PATCH 12/19] Keep workflow green when the expected-failures strict job fails Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JBbsEbiGm6MgTeRfCqdHi3 --- .github/workflows/ci_tests.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci_tests.yml b/.github/workflows/ci_tests.yml index 1d1951f8..2423fb71 100644 --- a/.github/workflows/ci_tests.yml +++ b/.github/workflows/ci_tests.yml @@ -121,10 +121,13 @@ jobs: # 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. + # without cancelling the main test matrix above. continue-on-error keeps + # the workflow (and the PR checks summary) green when this job fails; + # the job itself still shows its real pass/fail status. ci-tests-expected-failures: name: ${{ matrix.name }} runs-on: ${{ matrix.os }} + continue-on-error: true if: "!(contains(github.event.head_commit.message, '[skip ci]') || contains(github.event.head_commit.message, '[ci skip]'))" strategy: matrix: From b5648342d32e6403587d55b7c0f4edc6e8742184 Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Sun, 12 Jul 2026 11:10:22 -0500 Subject: [PATCH 13/19] Report strict-job status via neutral check instead of failing the rollup Job-level continue-on-error only greens the workflow run conclusion; the job's own check run still reports failure, so the PR rollup stayed red. Move continue-on-error to the test step (job and rollup go green), report the real outcome as a warning annotation and step-summary line, and upload it as an artifact that a new workflow_run workflow turns into a check run (neutral when failing, success when passing). The follow-up workflow runs from the default branch, so the extra check only appears once this merges. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01G6gS7Qb6gnFCmo6n7BvkW8 --- .github/workflows/ci_tests.yml | 25 ++++++++++-- .github/workflows/strict_status.yml | 62 +++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 4 deletions(-) create mode 100644 .github/workflows/strict_status.yml diff --git a/.github/workflows/ci_tests.yml b/.github/workflows/ci_tests.yml index 2423fb71..66ba95ae 100644 --- a/.github/workflows/ci_tests.yml +++ b/.github/workflows/ci_tests.yml @@ -121,13 +121,14 @@ jobs: # 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. continue-on-error keeps - # the workflow (and the PR checks summary) green when this job fails; - # the job itself still shows its real pass/fail status. + # 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 the strict-job-outcome artifact that strict_status.yml + # turns into a neutral check run on the PR. ci-tests-expected-failures: name: ${{ matrix.name }} runs-on: ${{ matrix.os }} - continue-on-error: true if: "!(contains(github.event.head_commit.message, '[skip ci]') || contains(github.event.head_commit.message, '[ci skip]'))" strategy: matrix: @@ -155,5 +156,21 @@ jobs: python --version python -m pip list - name: Run tests + id: tests + continue-on-error: true run: | tox -e ${{ matrix.tox_env }} -- ${{ matrix.toxposargs }} + - name: Record test outcome + run: | + echo "${{ steps.tests.outcome }}" > strict-outcome.txt + if [ "${{ steps.tests.outcome }}" = "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" + else + echo ":tada: **${{ matrix.name }}**: tests **passed** -- the expected-failures carve-out for this job can be retired" >> "$GITHUB_STEP_SUMMARY" + fi + - name: Upload test outcome + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # 6.0.0 + with: + name: strict-job-outcome + path: strict-outcome.txt diff --git a/.github/workflows/strict_status.yml b/.github/workflows/strict_status.yml new file mode 100644 index 00000000..ff9f7a24 --- /dev/null +++ b/.github/workflows/strict_status.yml @@ -0,0 +1,62 @@ +# Turn the outcome of the expected-failures strict array-API job into a +# check run 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. A failure shows up as a neutral (grey) check +# so the PR rollup stays green while the real status is still visible in the +# checks list. +# +# 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: + runs-on: ubuntu-latest + steps: + - name: Download strict job outcome + id: download + # The artifact is missing when the strict job did not run (e.g. the CI + # run was skipped); tolerate that and skip the check run below. + continue-on-error: true + uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # 7.0.0 + with: + name: strict-job-outcome + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ secrets.GITHUB_TOKEN }} + - name: Create check run + if: steps.download.outcome == 'success' + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + script: | + const fs = require('fs'); + const outcome = fs.readFileSync('strict-outcome.txt', 'utf8').trim(); + const passed = outcome === 'success'; + const run = context.payload.workflow_run; + await github.rest.checks.create({ + owner: context.repo.owner, + repo: context.repo.repo, + name: passed ? 'py313-strict: passing' : 'py313-strict: failing (expected)', + head_sha: run.head_sha, + status: 'completed', + conclusion: passed ? 'success' : 'neutral', + output: { + title: passed + ? 'Strict array-API tests passed' + : 'Strict array-API tests failed (expected)', + summary: passed + ? `The py313-strict job in [this CI run](${run.html_url}) passed. ` + + 'The expected-failures carve-out for it can be retired.' + : `The py313-strict 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.', + }, + }); From b00a459b6a308c494db806e90b331f8a1a07b103 Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Tue, 14 Jul 2026 10:43:53 -0500 Subject: [PATCH 14/19] Guard escape-baseline write/enforce modes against a disabled logger Both CCDPROC_WRITE_ESCAPE_BASELINE and CCDPROC_ENFORCE_ESCAPE_BASELINE read the sites tallied by the CCDPROC_LOG_ARRAY_ESCAPES logger, but nothing coupled them in code: write mode with the logger off silently truncated the baseline to an empty stub, and enforce mode vacuously passed while printing a reassuring message. Fail the session with a UsageError before any test runs when either mode is set without the logger, refuse to rewrite the baseline when zero escapes were observed, and warn loudly (listing the entries) when a rewrite drops baseline entries, since a subset run drops sites its tests never exercised. Also remove the stale jax backend_xfail on test_generator_ccds_without_unit: the ValueError it references is raised by fits_ccddata_reader before the backend conversion block, so the backend cannot affect it, and the test xpasses on jax. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01M5FNYqVHbKLbVSGJGftLrL --- ccdproc/conftest.py | 1 + ccdproc/tests/_escape_triage.py | 101 ++++++++++++++++++++++--- ccdproc/tests/test_image_collection.py | 5 -- 3 files changed, 93 insertions(+), 14 deletions(-) diff --git a/ccdproc/conftest.py b/ccdproc/conftest.py index 3ee53907..ec1d904d 100644 --- a/ccdproc/conftest.py +++ b/ccdproc/conftest.py @@ -28,6 +28,7 @@ def pytest_configure(config): _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, ) diff --git a/ccdproc/tests/_escape_triage.py b/ccdproc/tests/_escape_triage.py index 35524761..1415c890 100644 --- a/ccdproc/tests/_escape_triage.py +++ b/ccdproc/tests/_escape_triage.py @@ -42,6 +42,11 @@ def _env_truthy(value): #: 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) @@ -167,14 +172,33 @@ def _stale_baseline_entries(): 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. + 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: ", @@ -182,16 +206,16 @@ def _write_escape_baseline(): "# 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).", + "# 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.", "#", ] - if not sites: - _write_lines(header + ["# (no library escapes observed this run)"]) - return w_file = max(len(f) for f, _, _ in sites) w_func = max(len(fn) for _, fn, _ in sites) w_co = max(len(c) for _, _, c in sites) @@ -293,6 +317,7 @@ def pytest_terminal_summary(terminalreporter): _report_escape_failures(terminalreporter) _report_escape_log_counts(terminalreporter) _report_escape_baseline(terminalreporter) + _report_escape_baseline_dropped(terminalreporter) def _report_escape_failures(terminalreporter): @@ -365,13 +390,16 @@ def _report_escape_baseline(terminalreporter): if not ENFORCE_BASELINE: return - # No foreign arrays seen at all means the logger ran on a numpy backend - # (or was never installed). The baseline is about non-numpy backends, so + # 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( - "No foreign-array escapes observed (numpy backend?); " + "Escape logger was active but observed no foreign-array escapes " + "(numpy backend, or a subset run exercising no escape site); " "baseline not checked." ) return @@ -399,12 +427,67 @@ def _report_escape_baseline(terminalreporter): 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 pytest_sessionstart(session): # noqa: ARG001 fixed pytest hook signature + """ + Fail fast on unusable env-var combinations. 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. + """ + 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. In enforce mode + 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. diff --git a/ccdproc/tests/test_image_collection.py b/ccdproc/tests/test_image_collection.py index d120f6b7..c33d7ab7 100644 --- a/ccdproc/tests/test_image_collection.py +++ b/ccdproc/tests/test_image_collection.py @@ -383,11 +383,6 @@ def test_generator_data(self, triage_setup): for img in collection.data(): assert isinstance(img, np.ndarray) - @pytest.mark.backend_xfail( - "jax", - reason="ccds() on the jax backend does not raise ValueError when " - "the files have no unit", - ) def test_generator_ccds_without_unit(self, triage_setup): collection = ImageFileCollection( location=triage_setup.test_dir, keywords=["imagetyp"] From f3d583d7b16035417a92c94f2b81236c3a2209a9 Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Tue, 14 Jul 2026 10:44:09 -0500 Subject: [PATCH 15/19] Harden the strict-status reporting pipeline - Record the strict-job outcome as JSON with an explicit third state ("error") so an infrastructure failure (skipped/cancelled tests step, broken env creation) is reported as a red failure check instead of being conflated with "failing (expected)"; the record and upload steps run under if: always() so an artifact exists even when an earlier step fails. - Name the outcome artifact per matrix entry and download by pattern: upload-artifact v4+ errors on duplicate names, so the previous fixed name would break the moment the expected-failures matrix grew. - Use a stable check-run name (the matrix entry name) and carry the outcome in the conclusion and output instead of encoding it in the check name. - Only report on workflow runs triggered by pull_request, so same-repo PR branches (which trigger both push and pull_request CI runs) do not get duplicate, possibly contradictory, checks on the same head SHA. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01M5FNYqVHbKLbVSGJGftLrL --- .github/workflows/ci_tests.yml | 32 ++++++-- .github/workflows/strict_status.yml | 121 +++++++++++++++++++++------- 2 files changed, 115 insertions(+), 38 deletions(-) diff --git a/.github/workflows/ci_tests.yml b/.github/workflows/ci_tests.yml index 66ba95ae..5a1be706 100644 --- a/.github/workflows/ci_tests.yml +++ b/.github/workflows/ci_tests.yml @@ -124,8 +124,8 @@ jobs: # 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 the strict-job-outcome artifact that strict_status.yml - # turns into a neutral check run on the PR. + # 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 }} @@ -160,17 +160,35 @@ jobs: 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: | - echo "${{ steps.tests.outcome }}" > strict-outcome.txt - if [ "${{ steps.tests.outcome }}" = "failure" ]; then + 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" - else + 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: - name: strict-job-outcome - path: strict-outcome.txt + # 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 index ff9f7a24..4ded2a0f 100644 --- a/.github/workflows/strict_status.yml +++ b/.github/workflows/strict_status.yml @@ -1,9 +1,11 @@ -# Turn the outcome of the expected-failures strict array-API job into a -# check run on the PR. This runs as a separate workflow_run workflow because +# 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. A failure shows up as a neutral (grey) check -# so the PR rollup stays green while the real status is still visible in the -# checks list. +# 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. @@ -20,43 +22,100 @@ permissions: 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 outcome + - name: Download strict job outcomes id: download - # The artifact is missing when the strict job did not run (e.g. the CI - # run was skipped); tolerate that and skip the check run below. + # 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: - name: strict-job-outcome + 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 run + - name: Create check runs if: steps.download.outcome == 'success' uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: script: | const fs = require('fs'); - const outcome = fs.readFileSync('strict-outcome.txt', 'utf8').trim(); - const passed = outcome === 'success'; + const path = require('path'); const run = context.payload.workflow_run; - await github.rest.checks.create({ - owner: context.repo.owner, - repo: context.repo.repo, - name: passed ? 'py313-strict: passing' : 'py313-strict: failing (expected)', - head_sha: run.head_sha, - status: 'completed', - conclusion: passed ? 'success' : 'neutral', - output: { - title: passed - ? 'Strict array-API tests passed' - : 'Strict array-API tests failed (expected)', - summary: passed - ? `The py313-strict job in [this CI run](${run.html_url}) passed. ` + - 'The expected-failures carve-out for it can be retired.' - : `The py313-strict 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.', - }, - }); + 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 }, + }); + } From 115475d390b308f4efbb789beddeeb14bc656a34 Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Tue, 14 Jul 2026 10:44:22 -0500 Subject: [PATCH 16/19] Document the escape-baseline ratchet and fix changelog entries Add docs for the escape-baseline ratchet (what the baseline file is, how to regenerate it, what to do when the enforce CI job fires) and for reading the strict CI signal (the always-green carve-out job vs the separate check run posted by the strict-status workflow), and list the CCDPROC_ENFORCE_ESCAPE_BASELINE / CCDPROC_WRITE_ESCAPE_BASELINE env vars alongside the other developer tooling. Fix the changelog: both new entries belong to PR #942 (not #937/#939), and the section heading follows the x.y.z (unreleased) convention. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01M5FNYqVHbKLbVSGJGftLrL --- CHANGES.rst | 8 +++--- docs/array_api.rst | 71 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 833f7a36..6e3e2da0 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,15 +1,15 @@ -Unreleased ----------- +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. [#937] + 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``). [#939] + (``CCDPROC_LOG_ARRAY_ESCAPES=1``). [#942] 2.5.1 (2025-07-05) ------------------ diff --git a/docs/array_api.rst b/docs/array_api.rst index 8203059c..e8d24e5e 100644 --- a/docs/array_api.rst +++ b/docs/array_api.rst @@ -65,6 +65,77 @@ A few more developer tools help triage failures on non-numpy backends: 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. ++ 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? ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From 591a83b0998780888efd593a2fc1c817f7f1c75b Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Tue, 14 Jul 2026 11:20:15 -0500 Subject: [PATCH 17/19] Restore jax xfail on test_generator_ccds_without_unit Removing this marker as stale broke the ubuntu-py313-jax CI job: the test genuinely fails there (ccds() does not raise ValueError for unit-less files), and the previous CI run confirms the marker was xfailing in that environment all along. The staleness verification had been done on macOS, where the test passes on jax with identical astropy/jax/numpy versions, so the behavior is platform-dependent. Restore the marker with a reason that records what is actually known; the Linux-side root cause still needs investigation. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01M5FNYqVHbKLbVSGJGftLrL --- ccdproc/tests/test_image_collection.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ccdproc/tests/test_image_collection.py b/ccdproc/tests/test_image_collection.py index c33d7ab7..ca8f2878 100644 --- a/ccdproc/tests/test_image_collection.py +++ b/ccdproc/tests/test_image_collection.py @@ -383,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"] From 98a6ec1e3fff2992b028eba7d12cc27698d1c438 Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Tue, 14 Jul 2026 11:35:20 -0500 Subject: [PATCH 18/19] Document triage-tooling limitations and guard the ratchet against xdist Review follow-ups on PR #942 (items D1, D2, D4, D5, R4): - tox.ini: explain why the strict env installs no optional deps, why the CI strict/enforce envs omit the "test" factor, and why the enforce factor sets both escape-tooling variables. - _escape_triage.py / conftest.py: note that only Python-level module-attribute calls to np.asarray/np.asanyarray/np.ma.asanyarray are visible to the escape logger; C-level coercions and pre-patch imports are structural false negatives. - _escape_triage.py: reject baseline write/enforce modes under pytest-xdist -- the per-process escape tally never reaches the controller, so the baseline would be truncated or enforcement would pass vacuously. - docs/array_api.rst: backend_xfail markers are non-strict, so fixed backend bugs become silent XPASSes; check for them with -rX and confirm against CI logs (not just locally) before pruning a marker. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01M5FNYqVHbKLbVSGJGftLrL --- ccdproc/conftest.py | 6 ++++++ ccdproc/tests/_escape_triage.py | 38 +++++++++++++++++++++++++++++++-- docs/array_api.rst | 11 +++++++++- tox.ini | 16 ++++++++++++-- 4 files changed, 66 insertions(+), 5 deletions(-) diff --git a/ccdproc/conftest.py b/ccdproc/conftest.py index ec1d904d..5b0f5e68 100644 --- a/ccdproc/conftest.py +++ b/ccdproc/conftest.py @@ -159,6 +159,12 @@ def pytest_collection_modifyitems(items): # 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. # --------------------------------------------------------------------------- diff --git a/ccdproc/tests/_escape_triage.py b/ccdproc/tests/_escape_triage.py index 1415c890..b4db105e 100644 --- a/ccdproc/tests/_escape_triage.py +++ b/ccdproc/tests/_escape_triage.py @@ -7,6 +7,13 @@ (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 @@ -455,14 +462,41 @@ def _report_escape_baseline_dropped(terminalreporter): ) -def pytest_sessionstart(session): # noqa: ARG001 fixed pytest hook signature +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 env-var combinations. Both baseline modes read the + 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: " diff --git a/docs/array_api.rst b/docs/array_api.rst index e8d24e5e..1fcceeca 100644 --- a/docs/array_api.rst +++ b/docs/array_api.rst @@ -64,7 +64,16 @@ A few more developer tools help triage failures on non-numpy backends: + 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. + 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. diff --git a/tox.ini b/tox.ini index be17161f..943bcfe0 100644 --- a/tox.ini +++ b/tox.ini @@ -24,8 +24,10 @@ setenv = # 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 turns it on too; compose with a backend factor, e.g. - # py312-test-dask-enforce. Refresh the baseline locally (in the source + # 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 @@ -35,6 +37,12 @@ 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} @@ -77,6 +85,10 @@ 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 = From d329ac5c9df74d0b1a35c77881644fba54974923 Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Tue, 14 Jul 2026 14:58:23 -0500 Subject: [PATCH 19/19] Address inline review comments: comments, docstrings, width computation - _write_escape_baseline: collapse the three per-column max() comprehensions into one transpose over the site tuples. - pytest_collection_modifyitems: add a docstring explaining how the backend_skip/backend_xfail markers are applied at collection time. - _make_escape_logging_wrapper: document the wrapper's log-and-tally behavior and why the reentrancy guard is needed. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01M5FNYqVHbKLbVSGJGftLrL --- ccdproc/conftest.py | 30 ++++++++++++++++++++++++++++++ ccdproc/tests/_escape_triage.py | 9 ++++++--- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/ccdproc/conftest.py b/ccdproc/conftest.py index 5b0f5e68..c5c2ac3f 100644 --- a/ccdproc/conftest.py +++ b/ccdproc/conftest.py @@ -129,6 +129,17 @@ def _normalize_backend_name(name): 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} @@ -137,6 +148,7 @@ def pytest_collection_modifyitems(items): "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"): @@ -235,13 +247,31 @@ def __init__(self): 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( diff --git a/ccdproc/tests/_escape_triage.py b/ccdproc/tests/_escape_triage.py index b4db105e..973d48f7 100644 --- a/ccdproc/tests/_escape_triage.py +++ b/ccdproc/tests/_escape_triage.py @@ -223,9 +223,12 @@ def _write_escape_baseline(): "# that will never leave. Verify/adjust the seeded tags by hand.", "#", ] - w_file = max(len(f) for f, _, _ in sites) - w_func = max(len(fn) for _, fn, _ in sites) - w_co = max(len(c) for _, _, c in sites) + # 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