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

Allow pypy3, pypy3.6 or pypy3.7 syntax #161

Closed
wants to merge 2 commits into from
Closed

Conversation

mattip
Copy link

@mattip mattip commented Nov 9, 2020

fixes gh-136

This is my first PR so it is probably wrong. For instance, it seems the code in dist/* is generated from src/* ?

Also: will the versions be "automatically" downloaded into the image if this is accepted or is more work needed in another repo?

@mattip
Copy link
Author

mattip commented Nov 12, 2020

CI is passing

@mattip
Copy link
Author

mattip commented Nov 25, 2020

ping

@maxim-lobanov
Copy link
Contributor

Hello @mattip , sorry for delay with reviewing it.
This PR switches pypy3 alias to use Python 3.7 instead of 3.6 that probably will be breaking change for some customers.
Also, in current state, this action only uses pre-cached Python from images and can't install it on-flight. If we pre-cache both 3.6 and 3.7 on images, all previous versions of this action, will start to use 3.7 automatically instead of 3.6 and it will break customers.

We are thinking about more complex solution to provide better PyPy support in this action, like installing versions on-flight if they are not available in cache and etc. I will post some details to https://foss.heptapod.net/pypy/pypy/-/issues/3354. Thank you for your help

@jarrodmillman
Copy link

@maxim-lobanov Thanks for responding to this PR. It would be great to see support for pypy3.7 soonish. Recently several of the foundational projects in the scientific Python ecosystem have adopted "Recommend Python and Numpy version support as a community policy standard": https://numpy.org/neps/nep-0029-deprecation_policy.html

In particular, we have decided to drop support for Python 3.6. numpy, scipy, matplotlib, scikit-image, networkx, and others have either already dropped support for 3.6 or are in the process. Many of the projects have also started using GH actions for pypy ci testing. Most of the foundational projects provide some level of pypy support (even if we don't advertise it heavily) and we would like to stop testing on 3.6, but continue testing on pypy.

If there is anything we can do to help make this happen ASAP, please let us know.

@maxim-lobanov
Copy link
Contributor

@jarrodmillman , thank you for feedback.
Our team has already started to work on it so I think we will release support for it soon.
A few points where we would be appreciate for help from PyPy team:

@mattip
Copy link
Author

mattip commented Dec 2, 2020

JSON is available at https://downloads.python.org/pypy/versions.json. It includes the latest release.

@mattip
Copy link
Author

mattip commented Dec 2, 2020

This PR switches pypy3 alias to use Python 3.7 instead of 3.6

That is on purpose. People who want a specific version should specify it. Better to make this clear from now on, when only a few CI users have pypy running than later when it becomes more popular. The other option is to deprecate using only pypy3 as a specifier. People who want the old behavior can pin the version of setup-python.

@maxim-lobanov
Copy link
Contributor

@mattip , On the one hand, I agree. pypy3 is definitely should point to latest PyPy 3.x. On the other hand, it will impact customers that we would like to avoid. Looks like the first iteration of this action was not designed to support more than 2 versions in future. It is the reason why we would like to revisit it.

I think if we introduce additional field:

with:
   python-version: 3.x
   pypy-version: 7.x

it will be clear that 3.x will resolve latest version and customers won't be impacted in future when 3.8, 3.9 are available.

We would like to keep input pypy3 to point to 3.6 for backward compatibility. If we introduce new field and keep previous pypy3 keyword, it will work for us. We will mark pypy3 keyword as deprecated in new version and will ask customers to use new option but they won't be broken immediately.

@maxim-lobanov
Copy link
Contributor

Let me publish the full our proposal to be clear:

Select CPython (without PyPy)

uses: actions/setup-python@v2
with:
   python-version: 3.6

Select exact PyPy version

uses: actions/setup-python@v2
with:
   python-version: 3.6
   pypy-version: 7.3.3

Select PyPy version by partial inputs

uses: actions/setup-python@v2
with:
   python-version: 3.x
   pypy-version: 7.3.x // or 7.x

Select latest version of PyPy

uses: actions/setup-python@v2
with:
   python-version: x
   pypy-version: x

Select latest nightly build of PyPy

uses: actions/setup-python@v2
with:
   python-version: 2
   pypy-version: nightly

Backward compatibility with old inputs

uses: actions/setup-python@v2
with:
   python-version: pypy3 # continue pointing to 3.6

@nulano
Copy link

nulano commented Dec 2, 2020

How would introducing a second field work with the job matrix? Wouldn't it be simpler to follow the same format PyPy uses for relase numbers, e.g. python-version: pypy3.7-v.7.3.3? And you could still keep the x meaning latest version, e.g. pypy3.x-7.3.x. I think that would make it much easier to set up the job matrix, e.g.:

jobs:
  build:
    strategy:
      fail-fast: false
      matrix:
        os: [
          "ubuntu-latest",
          "macOS-latest",
        ]
        python-version: [
          "pypy3.x-nightly",
          "pypy3.7-v7.3.x",
          "pypy3.6-v7.3.x",
          "3.10-dev",
          "3.9",
          "3.8",
          "3.7",
          "3.6",
        ]
    steps:
    - uses: actions/setup-python@v2
      with:
         python-version: ${{ matrix.python-version }}

I'm not even sure what would be an easy way to achieve the same result using the proposed two field approach.

@maxim-lobanov
Copy link
Contributor

maxim-lobanov commented Dec 2, 2020

@nulano , thank you for your proposal.
Your option with the single python-version field looks really interesting and more convenient to use in the matrix with CPython.
I would just add extra - symbol after pypy:

"pypy-3.x-nightly", 
"pypy-3.7-v7.3.x",
"pypy-3.6-v7.3.x",
"pypy-2-v7.3.x",
"pypy-3.6", // means Python 3.6 and latest PyPy
"3.10-dev",
"3.9",
"3.8",

Extra - is needed to avoid collision with old format where only pypy2 and pypy3 keywords were supported.
As I mentioned above, we would like to keep backward compatibility (keep pypy3 pointing to 3.6 for now)

@mattip , @jarrodmillman , @konradpabjan what do you think about such input format?

@mattip
Copy link
Author

mattip commented Dec 2, 2020

looks good to me

@konradpabjan
Copy link
Collaborator

I like the idea! 🚀

My only minor concern is regarding back-compat with our existing pypy3 input. For now we can keep pypy3 pointing to 3.6 (I'm okay with that compromise). Down the line (when a v3 or any new major version is released) we could get rid of it or switch over to making it point to the latest. A note in the README should suffice though for now about it being "legacy" and how we're not changing it so we don't break users

@brcrista any thoughts?

@konradpabjan
Copy link
Collaborator

We've added support for different distributions of PyPy with the latest setup-python@v2 release. Thank for the initial work and discussions in this PR as it helped move us forward.

For available PyPy versions and the new syntax, see: https://github.com/actions/setup-python#specifying-a-pypy-version

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.

Support PyPy3.7 separate from PyPy3.6
5 participants