Add no-GIL interpreter support #641
Conversation
Add `pytest-run-parallel` as dependency, test no-GIL interpreters in CI, and mark Cython module as safe for freethreaded interpreters.
There was a problem hiding this comment.
Hey @clin1234! Thanks for working on this. I'm not the maintainer here, but I'm leaving a couple of comments to help move this forward.
| @@ -0,0 +1,51 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
According to #613 (comment), sharing Packer objects wouldn't be supported under free-threading, so this test is probably not needed. Could we run the test suite with pytest-run-parallel instead?
There was a problem hiding this comment.
Whatever works: I borrowed this test from another fork, and I'm still learning how to modernize existing pytest tests to use pytest-run-parallel
There was a problem hiding this comment.
What I would do is install pytest-run-parallel in CI and then add a step in the main test job that looks something like this:
- name: Install pytest-run-parallel under free-threading
if: matrix.py == '3.14t-dev' || matrix.py == '3.13t'
shell: bash
run: |
pip install pytest-run-parallel
- name: Run tests in parallel under free-threading
if: matrix.py == '3.14t-dev' || matrix.py == '3.13t'
shell: bash
run: |
pytest -v --parallel-threads=auto --iterations=20 test
Co-authored-by: Lysandros Nikolaou <[email protected]>
There was a problem hiding this comment.
Thanks for iterating on this so quickly @clin1234! Left some more comments on here. Hope these are helpful!
.github/workflows/test.yml
Outdated
| @@ -10,7 +10,7 @@ jobs: | |||
| strategy: | |||
| matrix: | |||
| os: ["ubuntu-latest", "windows-latest", "macos-latest"] | |||
| py: ["3.14-dev", "3.13", "3.12", "3.11", "3.10", "3.9"] | |||
| py: ["3.14-dev", "3.14t-dev", "3.13", "3.13t", "3.12", "3.11", "3.10", "3.9", "3.8"] | |||
There was a problem hiding this comment.
Not sure if adding 3.8 here is a good idea, since it's unrelated to the purpose of the PR.
requirements.txt
Outdated
| @@ -1 +1,2 @@ | |||
| Cython~=3.1.2 | |||
| pytest-run-parallel[psutil] | |||
There was a problem hiding this comment.
It looks like pytest is not listed as a requirement here, so maybe it's better to remove pytest-run-parallel from here and install it in CI directly?
| @@ -17,7 +17,7 @@ def test_unpack_bytearray(): | |||
| obj = unpackb(buf, use_list=1) | |||
| assert [b"foo", b"bar"] == obj | |||
| expected_type = bytes | |||
| assert all(type(s) == expected_type for s in obj) | |||
| assert all(type(s) is expected_type for s in obj) | |||
There was a problem hiding this comment.
These also seem unrelated to the purpose of the PR (adding support for free-threading).
| @@ -0,0 +1,51 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
What I would do is install pytest-run-parallel in CI and then add a step in the main test job that looks something like this:
- name: Install pytest-run-parallel under free-threading
if: matrix.py == '3.14t-dev' || matrix.py == '3.13t'
shell: bash
run: |
pip install pytest-run-parallel
- name: Run tests in parallel under free-threading
if: matrix.py == '3.14t-dev' || matrix.py == '3.13t'
shell: bash
run: |
pytest -v --parallel-threads=auto --iterations=20 testOn Windows, attempting to upgrade `pip` within CI always yields this: ``` Run pip install -U pip Requirement already satisfied: pip in c:\hostedtoolcache\windows\python\3.10.11\x64\lib\site-packages (25.1.1) Collecting pip Downloading pip-25.2-py3-none-any.whl.metadata (4.7 kB) Downloading pip-25.2-py3-none-any.whl (1.8 MB) ---------------------------------------- 1.8/1.8 MB 48.1 MB/s eta 0:00:00 Notice: A new release of pip is available: 25.1.1 -> 25.2 Notice: To update, run: python.exe -m pip install --upgrade pip ERROR: To modify pip, please run the following command: C:\hostedtoolcache\windows\Python\3.10.11\x64\python.exe -m pip install -U pip Error: Process completed with exit code 1. ```
No description provided.