Skip to content

ENH: Free-threading support #330

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 17, 2025
Merged

ENH: Free-threading support #330

merged 5 commits into from
Jun 17, 2025

Conversation

crusaderky
Copy link
Contributor

@crusaderky crusaderky commented Jun 10, 2025

Matching scipy PR:

Demo

tests/test_testing.py::test_lazy_xp_function[array_api_strict] PARALLEL PASSED                                                                                       [ 99%]
tests/test_testing.py::test_lazy_xp_function[array_api_strictest] PARALLEL PASSED                                                                                    [ 99%]
tests/test_testing.py::test_lazy_xp_function[numpy] PARALLEL PASSED                                                                                                  [ 99%]
tests/test_testing.py::test_lazy_xp_function[numpy:readonly] PARALLEL PASSED                                                                                         [ 99%]
tests/test_testing.py::test_lazy_xp_function[cupy] SKIPPED (could not import 'cupy': No module named 'cupy')                                                         [ 99%]
tests/test_testing.py::test_lazy_xp_function[torch] SKIPPED (could not import 'torch': No module named 'torch')                                                      [ 99%]
tests/test_testing.py::test_lazy_xp_function[torch:gpu] SKIPPED (could not import 'torch': No module named 'torch')                                                  [ 99%]
tests/test_testing.py::test_lazy_xp_function[dask] PASSED [thread-unsafe]: uses thread_unsafe marker                                                                 [ 99%]
tests/test_testing.py::test_lazy_xp_function[sparse] SKIPPED (could not import 'sparse': No module named 'sparse')                                                   [ 99%]
tests/test_testing.py::test_lazy_xp_function[jax] SKIPPED (could not import 'jax.numpy': No module named 'jax')                                                      [ 99%]
tests/test_testing.py::test_lazy_xp_function[jax:gpu] SKIPPED (could not import 'jax.numpy': No module named 'jax')                                                  [ 99%]

@crusaderky crusaderky force-pushed the nogil branch 4 times, most recently from efde2aa to bf5b21c Compare June 10, 2025 11:37
@crusaderky crusaderky changed the title TST: Free-threaded tests TST: Free-threading support Jun 10, 2025
@crusaderky crusaderky changed the title TST: Free-threading support ENH: Free-threading support Jun 10, 2025
@@ -23,6 +23,7 @@ classifiers = [
"Programming Language :: Python :: 3.11",
"Programming Language :: Python :: 3.12",
"Programming Language :: Python :: 3.13",
"Programming Language :: Python :: Free Threading :: 3 - Stable",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole library has 1 stateful class, at, which has trivial state. So no point adding tests beyond those of pytest-run-parallel.
Where we are thread-unsafe, in lazy_xp_function, it is clearly documented, so I believe this should qualify for the Stable badge?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where we are thread-unsafe, in lazy_xp_function, it is clearly documented, so I believe this should qualify for the Stable badge?

@ngoldbaum perhaps you could confirm?

@lucascolley lucascolley added the enhancement New feature or request label Jun 11, 2025
@lucascolley lucascolley added this to the 0.8.1 milestone Jun 11, 2025
Copy link
Member

@lucascolley lucascolley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks Guido!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would appreciate a second review from someone for the changes in this file. All looks fine to me from a non-expert perspective, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping @rgommers

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just browsed the diff, looks good at a high level. I won't be able to do a line-by-line review, but that's probably also not necessary - and Nathan also just looked at the test machinery here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good then, happy to trust Guido on the details 👍

Copy link

@ngoldbaum ngoldbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ngoldbaum perhaps you could confirm?

Makes sense to me!

I didn't look at the pytest changes in detail but I looked it over to get a feel for what's happening here and it makes sense to me. I left one comment inline but it's not actionable by Guido. More a note to us that we should think about how to allow this testing pattern to work with pytest-run-parallel.

)
if self.like(Backend.DASK, Backend.JAX):
# Monkey-patched by lazy_xp_function
marks.append(pytest.mark.thread_unsafe)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is unavoidable but it's a little bit of a bummer. Makes me think as a long-term item we should figure out how to get e.g. mock or other monkeypatching to work in a thread-safe manner.

@lucascolley lucascolley merged commit dfa9808 into data-apis:main Jun 17, 2025
10 checks passed
@crusaderky crusaderky deleted the nogil branch June 17, 2025 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: lazy_xp_function is incompatible with pytest-run-parallel
4 participants