Skip to content
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

Support free threaded Python versions like '3.13t' #973

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

colesbury
Copy link

Python wheels, pyenv, and a number of other tools use 't' in the Python version number to identify free threaded builds. For example, '3.13t', '3.14.0a1', '3.14t-dev'.

This PR supports that syntax in actions/setup-python, strips the "t", and adds "-freethreading" to the architecture to select the correct Python version.

See #771 and actions/python-versions#319

Python wheels, pyenv, and a number of other tools use 't' in the Python
version number to identify free threaded builds. For example, '3.13t',
'3.14.0a1', '3.14t-dev'.

This PR supports that syntax in `actions/setup-python`, strips the "t",
and adds "-freethreading" to the architecture to select the correct
Python version.

See actions#771
@renxida
Copy link

renxida commented Nov 24, 2024

Meanwhile, if you can't wait, here at https://github.com/nod-ai/shark-ai/actions/workflows/ci_linux_x64_nogil-libshortfin.yml

we are using

    - name: Setup Python
      uses: deadsnakes/action@e640ac8743173a67cca4d7d77cd837e514bf98e8 # v3.2.0
      with:
        python-version: "3.13-dev"
        nogil : true

@rgommers
Copy link

rgommers commented Nov 24, 2024

If anyone needs the functionality in this PR, I'd recommend to use https://github.com/Quansight-Labs/setup-python. That is a fork of this action with the sole purpose of providing the functionality in this PR (free-threading support) until this PR gets merged (see this issue for context).

Data point for reviewers: we've exercised this PR quite heavily by now in projects like NumPy and SciPy, on Linux, macOS and Windows (x86-64 and aarch64/arm64), and it's been working smoothly to date.

@priyagupta108
Copy link
Contributor

Hi @colesbury 👋,
Thank you for your valuable contribution!
During our testing, we observed that the changes in this PR do not support specifying the patch version (e.g., 3.13.1t) for the python-version input for the free-threaded binary. Please ensure that the python-version input values align with the existing input syntax.
Additionally, To keep the documentation up-to-date, please update this PR to incorporate the corresponding documentation updates.

Thank you!

@ddelange
Copy link

Also noting here that 3.13 and 3.13t currently don't live happily next to each other on Windows: Quansight-Labs#5 (comment)

@colesbury
Copy link
Author

Hi @priyagupta108, thank you for reviewing the PR. I've fixed the handling of versions like 3.13.1t and added a test for that. I've also updated the README and "advanced-usage" documentation.

@priyagupta108
Copy link
Contributor

priyagupta108 commented Jan 29, 2025

Hi @colesbury 👋,

Thank you for making the updates! During our testing of this PR, we've identified a few issues related to free-threaded Python version inputs:

  • Prerelease Versions: The current changes do not support semver notation for prerelease versions such as alpha, beta, and RC. For example, versions like 3.14.0-alpha.1, 3.14.0-beta.2, and 3.14.0-rc.1 should be supported.
    Additionally, versions such as 3.14.0a1t and 3.14.0ta1 are mentioned in the documentation changes, but they are failing to run as shown in the screenshot below. Please note that the versions 3.14.0a1t and 3.14.0ta1 do not appear to be valid semver notation according to the semver guidelines.

    Screenshot 2025-01-29 at 11 38 26 AM
    Screenshot 2025-01-29 at 11 38 55 AM

  • Version Ranges: Various range syntaxes (e.g., hyphen ranges, x-ranges) are unsupported.

  • python-version-file Input: Unable to read the Python version from pyproject.toml. It seems that the python-version-file input is not functioning correctly, as shown in the below screenshot.

    Screenshot 2025-01-28 at 6 21 11 PM

Here is a demo run showcasing prerelease versions, version ranges, and python-version-file input examples. Please ensure that the PR changes support these input syntaxes for free-threaded Python versions.

To ensure full compatibility with all supported Python version formats and inputs including python-version-file input, please address these issues and ensure the implementation fully supports semantic versioning (semver) guidelines to handle different version formats correctly. Additionally, update the documentation to provide clarity on the syntax for free-threaded Python versions.
For details on supported Python version inputs, please refer the action's documentation.

Thank you!

@colesbury
Copy link
Author

Hi @priyagupta108, thanks for reviewing the PR again.

  • I've added an optional "freethreaded" input. As you point out, the "3.13t" syntax is not semver notation and there isn't a good way to represent it using semver. Users can specify "freethreading: true" and specify the version using semver
  • Prerelease notation like "3.14.0a4t" now works. I removed the "3.13.0ta4" notation because that's not supported by other Python tools.
  • Version ranges: These now work, but require setting the "freethreaded: true" input to get the freethreaded Python version.
  • python-version-file input: This also works and requires setting "freethreaded: true" to get the freethreaded Python version

I've included examples in "advanced-usage.md".

Thank you for the demo run. I've modified it and run it with these changes here:
https://github.com/colesbury/sample-python/actions/runs/13060324198

@colesbury
Copy link
Author

Hi @priyagupta108, I've made an additional change:

  • The outputs.python-version now includes the "t" suffix for free threaded builds. We've found that the python-version output is commonly used for cache keys (see this github search) and having a different identifier helps avoid problems with mixing caches between incompatible Python builds.

@priyagupta108
Copy link
Contributor

priyagupta108 commented Feb 10, 2025

Hi @colesbury,
Thank you for the prompt updates. We have reviewed the changes and have a few concerns:

  • Regarding new notation: The prerelease notation "3.14.0a4" introduced does not comply with the Semantic Versioning (SemVer) specification and is not supported by any setup-actions. Introducing it here could lead to inconsistencies across all actions. We recommend not including this notation in both the free-threaded and default Python version inputs to ensure that only valid SemVer formats are supported across all setup-actions.

  • Cache-key conflicts: The outputs.python-version correctly reflects the Python version according to the binary type. However, to avoid cache conflicts between free-threaded and default builds when using the setup-python cache feature, the cache key should be unique for each. Could you please make the necessary adjustments for the free-threaded cache key?
    Here are the demonstration runs showcasing the caching feature with setup-python:

Please feel free to reach out if you need any further clarification or assistance.

@ddelange
Copy link

  • However, to avoid cache conflicts between free-threaded and default builds when using the setup-python cache feature, the cache key should be identical for both.

I'd argue that the cache key be should be unique in order to avoid conflicts: I don't want to restore a 3.13t venv on a 3.13 runner or vice versa.

Furthermore, for pre-commit related issues, I don't even want to restore a 3.13.0 venv on a 3.13.1 runner (patch updates I get automatically when specifying 3.13 in setup-python, which I do want).

Solution is to simply use the setup-python output ${{ env.pythonLocation }} in the cache key, which is always unique and always safe.

@priyagupta108
Copy link
Contributor

@ddelange 👋, Sorry for the confusion and thanks for pointing it out. Yes, the cache key should be unique. I have edited my previous comment.

@colesbury
Copy link
Author

Hi @priyagupta108 - I've fixed the cache key conflicts and removed support for notation like "3.14.0a4".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants