Port requirements to PEP735 #29281
Port requirements to PEP735 #29281
Conversation
github-actions
bot
added
CI: Run cygwin
|
Note, |
There was a problem hiding this comment.
I was curious about this new PEP, so I took a look through the changes. I think this would be a really neat feature when pip implements it. I know this is just a draft, but I figured I can point out a couple things I noticed, and I had a couple of questions, too.
| @@ -174,10 +174,9 @@ jobs: | |||
| - name: Install Python dependencies | |||
| shell: bash.exe -eo pipefail -o igncr "{0}" | |||
| run: | | |||
| python -m pip install --upgrade pip setuptools wheel | |||
| python -m pip install --group build | |||
| python -m pip install kiwisolver 'numpy>=1.22,<1.26' pillow importlib_resources | |||
| grep -v -F -e psutil requirements/testing/all.txt >requirements_test.txt | |||
There was a problem hiding this comment.
requirements/testing/all.txt is removed
There was a problem hiding this comment.
Cygwin doesn't run right now, so I kind of gave up on finishing it; there are probably more missed entries there.
| @@ -71,8 +71,7 @@ stages: | |||
|
|
|||
| - bash: | | |||
| python -m pip install --upgrade pip | |||
| python -m pip install --upgrade -r requirements/dev/build-requirements.txt | |||
| python -m pip install -r requirements/testing/all.txt -r requirements/testing/extra.txt | |||
| python -m pip install --group build --group test --group test-extra | |||
There was a problem hiding this comment.
We'll have to see if it matters whether or not we were upgrading certain dependencies or not. I suspect not.
| # pip install --no-build-isolation --editable .[dev] | ||
| # | ||
| # Install the documentation requirements with: | ||
| # pip install -r requirements/doc/doc-requirements.txt |
There was a problem hiding this comment.
missed pip command to update
|
|
||
| # Extra pip requirements for the Python 3.10+ builds | ||
| test-extra = [ | ||
| "--prefer-binary", |
There was a problem hiding this comment.
Will command-line arguments and sys.platform references work in this PEP? It isn't entirely clear to me.
There was a problem hiding this comment.
Ah, the command-line argument probably won't, but we'll have to see how the pip implementation ends up. The sys.platform should work as these are standard dependency specifiers.
There was a problem hiding this comment.
I learned something new! Neat!
| @@ -51,7 +51,7 @@ jobs: | |||
| python-version: '3.10' | |||
|
|
|||
| - name: Install mypy | |||
| run: pip3 install -r requirements/testing/mypy.txt -r requirements/testing/all.txt | |||
| run: pip3 install --group typing | |||
There was a problem hiding this comment.
does this also need group "test"?
There was a problem hiding this comment.
I'm hoping not, so we can reduce the install set.
| # | ||
| # You will first need a matching Matplotlib installation | ||
| # e.g (from the Matplotlib root directory) | ||
| # pip install --no-build-isolation --editable .[dev] |
There was a problem hiding this comment.
Is the .[dev] correct? Or should it be just .?
There was a problem hiding this comment.
The [dev] is an extra, not a dependency group. It was intended to cover both build and runtime dependencies. It didn't really work though, as pip installed . before its dependencies, so you couldn't get the build ones early enough. Based on the other implementations, dependency groups should be strictly better, since they should install before ., but we'll have to see how it's finally implemented. I should probably rewrite this section a bit once that's finalized.
There was a problem hiding this comment.
Oh, I see. I'm not used to seeing an extra be specified against a directory, so I thought it was a typo. And, yeah, extras wouldn't work for installing dependencies.
PR summary
PEP735 introduces the concept of "Dependency Groups"; these essentially replace our use of various
requirements.txtfiles.NOTE: This is waiting for
pipto actually implement Dependency Groups: pypa/pip#12963 We may or may not wish to wait for that to bake a bit even after it's been implemented as well.PR checklist