Adopt the array api #885

mwcraig · 2025-03-19T15:17:48Z

This PR makes the necessary changes to adopt the array API. There are still some things to sort out:

Run tests mostly against numpy, but do one run against jax (or some other non-numpy array library)
Check for remaining invocations of numpy in the code.
Check whether changing .data_arr.mask to .data_arr_mask constitutes an API change. If it does (and I think it does) then patch things up so that .data_arr.mask points to .data_arr_mask
Clean up any remaining handling of immutable arrays (array_api_extra makes handling those easy)

Once those are done the next step will be to find an outside reviewer for this...

codecov · 2025-03-20T15:04:20Z

Codecov Report

❌ Patch coverage is 95.28302% with 20 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@5aa9219). Learn more about missing BASE report.
⚠️ Report is 13 commits behind head on main.

Files with missing lines	Patch %	Lines
ccdproc/_ccddata_wrapper_for_array_api.py	86.72%	15 Missing ⚠️
ccdproc/core.py	97.94%	3 Missing ⚠️
ccdproc/image_collection.py	92.30%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #885   +/-   ##
=======================================
  Coverage        ?   97.03%           
=======================================
  Files           ?        8           
  Lines           ?     1518           
  Branches        ?        0           
=======================================
  Hits            ?     1473           
  Misses          ?       45           
  Partials        ?        0

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

eteq · 2025-06-24T21:51:22Z

As a quick FYI I ran the test suite in this PR on a machine with a NVidia GPU with CCDPROC_ARRAY_LIBRARY=cupy and got a whole pile of errors (sent the log directly to @mwcraig but can also share snippets here if someone else wants to see them)

pyproject.toml

ccdproc/combiner.py

nstarman · 2025-06-25T18:23:58Z

pyproject.toml

@@ -8,17 +8,19 @@ dynamic = ["version"]
 description = "Astropy affiliated package"
 readme = "README.rst"
 license = { text = "BSD-3-Clause" }
-requires-python = ">=3.8"
+requires-python = ">=3.11"


Adopt Spec 0? Has that been adopted?

Good catch!

Wait, misread this -- what is Spectrum 0?

https://scientific-python.org/specs/spec-0000/

So the move here would be to bump it up to 3.12?

3.11 for a few more months. Then 3.12. I was also thinking about adding the badge saying "Spec 0", e.g. you can see in the README in https://github.com/GalacticDynamics/unxt

pyproject.toml

ccdproc/combiner.py

nstarman · 2025-06-26T22:49:40Z

pyproject.toml

@@ -8,17 +8,19 @@ dynamic = ["version"]
 description = "Astropy affiliated package"
 readme = "README.rst"
 license = { text = "BSD-3-Clause" }
-requires-python = ">=3.8"
+requires-python = ">=3.11"


https://scientific-python.org/specs/spec-0000/

ccdproc/core.py

pyproject.toml

These are necessary because upstream dependencies (astropy) explicitly check for numpy arrays rather than using the array API.

This is necessary because CCDData does not implement the array API currently.

One more fix FIx the fix

Turns out there were several places where the functions in the array's namespace were not actually being used. This fixes that. Clean up uncertainty wrapper

nstarman

Hi @mwcraig. LMK how I can be of help with reviews. This PR looks great, and it seems to be mostly down to small line-by-line suggestions.

pyproject.toml

ccdproc/tests/test_cosmicray.py

mwcraig · 2025-07-17T21:34:55Z

Hi @mwcraig. LMK how I can be of help with reviews. This PR looks great, and it seems to be mostly down to small line-by-line suggestions.

Thanks, @nstarman! I'll want one last look once I've tied down a few loose ends. One thing I learned since the coordination meeting is that using np.testing.assert_allclose is a bad idea because it converts any array-like arguments into numpy arrays for the comparison, hiding any mis-match in array name space between the two things being compared. As a result, there are still a few places that I thought were working that were actually converting the data or uncertainty or mask to numpy arrays.

I've switch to using xp.all(xpx.isclose(a, b)) for numerical array comparisons, which raises an exception if a and b are in different namespaces.

I'll ping you when I'm confident that I have all of those cases fixed up.

The former is not part of the array API but the latter is.

Co-authored-by: Nathaniel Starkman <[email protected]>

Copilot

Pull Request Overview

This PR makes the necessary changes to adopt the array API across ccdproc. The implementation allows users to leverage different array libraries (JAX, Dask, CuPy) while maintaining backward compatibility with numpy. Key infrastructure is added to support array API operations without changing existing user interfaces.

Comprehensive adoption of array API across core processing functions
Testing infrastructure for multiple array libraries (numpy, JAX, dask)
Updated documentation including new array API guide and examples

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tox.ini	Adds test environments for JAX and Dask array libraries with proper configuration
pyproject.toml	Updates dependencies to include array API packages and minimum version requirements
docs/index.rst	Adds array API documentation section to table of contents
docs/array_api.rst	New comprehensive guide explaining array API usage and library options
ccdproc/core.py	Major refactoring to support array API operations throughout processing functions
ccdproc/conftest.py	Test infrastructure for selecting array libraries via environment variables
ccdproc/tests/*.py	Extensive test updates to work with array API and multiple backends

Comments suppressed due to low confidence (1)

ccdproc/tests/test_combiner.py:746

There is a spelling error: 'combiune' should be 'combine'.

"""

Copilot · 2025-07-29T21:53:26Z

docs/array_api.rst

+with `sparse`_. A `pull request <https://github.com/astropy/ccdproc/pulls>`_ to FIX
+that would be a welcome contribution to the project.
+
+What limitations shuold I be aware of?


There is a spelling error: 'shuold' should be 'should'.

Suggested change

What limitations shuold I be aware of?

What limitations should I be aware of?

Copilot · 2025-07-29T21:53:27Z

docs/array_api.rst

+with `sparse`_. A `pull request <https://github.com/astropy/ccdproc/pulls>`_ to FIX
+that would be a welcome contribution to the project.


The word 'FIX' appears to be a TODO placeholder that should be replaced with an actual link or removed.

Suggested change

with `sparse`_. A `pull request <https://github.com/astropy/ccdproc/pulls>`_ to FIX

that would be a welcome contribution to the project.

with `sparse`_. A `pull request <https://github.com/astropy/ccdproc/pulls>`_ to add

support for `sparse`_ would be a welcome contribution to the project.

Copilot · 2025-07-29T21:53:27Z

docs/index.rst

@@ -34,6 +34,7 @@ Detailed, step-by-step guide

 In addition to the documentation here, a detailed guide to the topic of CCD
 data reduction using ``ccdproc`` and other `astropy`_ tools is available here:
+(TODO: FIX LINK BELOW)


There is a TODO comment indicating that the link below needs to be fixed. This should be addressed before merging.

Suggested change

(TODO: FIX LINK BELOW)

Copilot · 2025-07-29T21:53:27Z

ccdproc/tests/test_combiner.py

@@ -334,6 +340,7 @@ def test_combiner_mask_average():
    # are masked?!
    # assert ccd.data[0, 0] == 0
    assert ccd.data[5, 5] == 1
+    # THE LINE BELOW IS CATCHING A REAL ERROR


[nitpick] This comment suggests there may be an unresolved issue with the test. Consider adding more context about what error is being caught and whether it needs to be fixed.

Suggested change

# THE LINE BELOW IS CATCHING A REAL ERROR

# Ensure that the mask is correctly applied to pixels that are fully masked

Copilot · 2025-07-29T21:53:27Z

pyproject.toml

@@ -8,17 +8,19 @@ dynamic = ["version"]
 description = "Astropy affiliated package"
 readme = "README.rst"
 license = { text = "BSD-3-Clause" }
-requires-python = ">=3.8"
+requires-python = ">=3.11"


The minimum Python version has been increased from 3.8 to 3.11. This is a breaking change that should be clearly documented in the release notes.

Copilot · 2025-07-29T21:53:28Z

tox.ini

@@ -47,10 +50,11 @@ deps =

    # Remember to transfer any changes here to setup.cfg also. Only listing
    # packages which are constrained in the setup.cfg


The oldest supported numpy version has been changed from 1.24 to 1.26. This is a breaking change that should be documented.

Suggested change

# packages which are constrained in the setup.cfg

# packages which are constrained in the setup.cfg

# Note: The oldest supported version of NumPy has been updated from 1.24 to 1.26.

# This is a breaking change and should be taken into account when updating dependencies.

mwcraig added this to the 2.5.0 milestone Mar 19, 2025

mwcraig added the enhancement label Mar 19, 2025

mwcraig force-pushed the explore-array-api branch from 85625f0 to 04293b7 Compare March 27, 2025 18:05

mwcraig marked this pull request as ready for review June 24, 2025 14:55

mwcraig changed the title ~~WIP: Adopt the array api~~ Adopt the array api Jun 24, 2025

mwcraig requested a review from Copilot June 24, 2025 16:42

This comment was marked as outdated.

Sign in to view

nstarman reviewed Jun 25, 2025

View reviewed changes

nstarman reviewed Jun 26, 2025

View reviewed changes

ccdproc/core.py Outdated Show resolved Hide resolved

pyproject.toml Show resolved Hide resolved

mwcraig force-pushed the explore-array-api branch 3 times, most recently from 2fa76b7 to 3b2b9fc Compare July 3, 2025 18:28

mwcraig added 15 commits July 5, 2025 10:05

Initial attempt to adopt array API

6dd9166

A couple more changes

d3b6203

Add function for check if something is an array

f0bc6b8

Remove numpy from core

424ebdc

WIP rewrite of a boolean index

7f5050d

WIP testing updates

6a6bfa2

Change more almost_equal to allclose

1d2d171

Use the random number generator from numpy

22bc04d

Remove more in-place array operations

245d641

Set a reasonable tolerance value in float comparisons

55b90be

Continue to use numpy arrays in a few places

23de261

These are necessary because upstream dependencies (astropy) explicitly check for numpy arrays rather than using the array API.

Refactor warning handling for numerical warnings

dffadce

Avoid explicit use of numpy masked arrays

12ff8b0

Rewrite test to not modify array in-place

63f81ba

Use assert_allclose instead of older alternatives

4ae3fc6

mwcraig added 2 commits July 12, 2025 13:25

Make sure to use CCDData.data instead of CCDData in comparisons

bbc1249

This is necessary because CCDData does not implement the array API currently.

Add more robust handling of open file test

6af1e1e

One more fix FIx the fix

mwcraig force-pushed the explore-array-api branch from 8137626 to 6af1e1e Compare July 12, 2025 20:25

mwcraig added 7 commits July 12, 2025 16:27

Add several workarounds for non-compliance of CCData with Array API

5ec5759

Minor changes

41e17e6

Add and use wrapper classes to ensure array API use

7664bc0

Turns out there were several places where the functions in the array's namespace were not actually being used. This fixes that. Clean up uncertainty wrapper

Fix for immutable array types

fb6e7d6

Ensure array namespace is used throughout core tests

c4c1277

Include more uncertainty types in tests

048f079

Use array API correctly in combiner tests

359126a

nstarman reviewed Jul 17, 2025

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

ccdproc/tests/test_cosmicray.py Outdated Show resolved Hide resolved

mwcraig added 7 commits July 17, 2025 17:06

Add array namespace input to combine function

bb42d5b

Add an array namespace conversion in one more place

ba83726

Add argument for desired array namespace

698c6fa

Specify namespace in a couple of tests where data is read from disk

7c7ce31

Fix broken links in docstrings

9a5ccd8

Switch more tests away from np.testing or explain why not switching

4503ddf

Minimal array API docs

9afb5a6

mwcraig force-pushed the explore-array-api branch from 747d4a4 to 9afb5a6 Compare July 29, 2025 15:00

mwcraig modified the milestones: 2.5.0, 3.0 Jul 29, 2025

mwcraig and others added 6 commits July 29, 2025 16:21

Improve test coverage

5289ddc

Replace .array with .asarray

802d9a4

The former is not part of the array API but the latter is.

Apply suggestions from code review

126d174

Co-authored-by: Nathaniel Starkman <[email protected]>

Remove reference to list that is not used

ad9b9b5

Point to documentation for list of supported libraries

fd9f03e

Avoid more numpy arrays in tests

749761f

mwcraig requested a review from Copilot July 29, 2025 21:52

Copilot AI reviewed Jul 29, 2025

View reviewed changes

	What limitations shuold I be aware of?
	What limitations should I be aware of?

		with `sparse`_. A `pull request <https://github.com/astropy/ccdproc/pulls>`_ to FIX
		that would be a welcome contribution to the project.

	# THE LINE BELOW IS CATCHING A REAL ERROR
	# Ensure that the mask is correctly applied to pixels that are fully masked

		@@ -47,10 +50,11 @@ deps =

		# Remember to transfer any changes here to setup.cfg also. Only listing
		# packages which are constrained in the setup.cfg

Uh oh!

Adopt the array api #885

Are you sure you want to change the base?

Adopt the array api #885

Uh oh!

Conversation

mwcraig commented Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment was marked as outdated.

Uh oh!

eteq commented Jun 24, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nstarman Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

mwcraig Jun 26, 2025

Choose a reason for hiding this comment

Uh oh!

mwcraig Jun 26, 2025

Choose a reason for hiding this comment

Uh oh!

nstarman Jun 26, 2025

Choose a reason for hiding this comment

Uh oh!

mwcraig Jun 26, 2025

Choose a reason for hiding this comment

Uh oh!

nstarman Jun 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nstarman Jun 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nstarman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mwcraig commented Jul 17, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mwcraig commented Mar 19, 2025 •

edited

Loading

codecov bot commented Mar 20, 2025 •

edited

Loading