-
-
Notifications
You must be signed in to change notification settings - Fork 530
Revert to supporting simple Python factors #2849
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
Conversation
Instead of parsing factors with virtualenv's 'PythonSpec.from_string_spec', use our own regex to support the simple factors that have been supported since tox v1. Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: tox-dev#2657 Fixes: tox-dev#2848
e001896
to
4e64555
Compare
Other points:
|
We do want this. This will make Github actions simple to setup because you can use the same spec for loading the python version in github as the env you need to run. Removing the gymanstic needed for tox 3 to map 3.8 to py38 👍 This feature will make CI configs much simpler so I'll need to insist on it. See e.g. https://github.com/tox-dev/tox/blob/main/.github/workflows/check.yml#L20-L25 and then https://github.com/tox-dev/tox/blob/main/.github/workflows/check.yml#L43-L50 we need to do just because tox does not support
Today perhaps. There's also rustpython. I don't want to be the gatekeeper of what's reasonable python implementation. We also have ironpython and JVM pythons too. Most people don't use this, but I want tox to allow using these. We need to be permissive to make people's lives easier, even if they are not in majority. |
Okay, so we need to incorporate periods into the env names. Given it doesn't break the hyphens as delimiters idea, I could live with that.
(BTW,
So we can't use a wildcard because that would falsely identify e.g. [testenv:py38]
base_python = rustpython3.8 We could also introduce a top-level or testenv-level [tox]
python_interpreter = rustpython
[testenv]
commands = ... Would this be a reasonable compromise: support |
Oh, wait, you're arguing for
It's also unnecessary. For one, we could easily do this via a plugin like the old [tox]
testenv = py, linters, coverage, docs, typing jobs:
test:
runs-on: ubuntu-22.04
strategy:
matrix:
py:
- "3.11"
- "3.10"
- "3.9"
- "3.8"
- "3.7"
steps:
- name: Setup Python
uses: actions/setup-python@v4
with:
python-version: ${{ matrix.py }}
- name: Install tox
run: python -m pip install tox
- name: Run tests
run: tox Is this somehow insufficient? I mean, couldn't we document this as our recommended practice? |
Another point on this: this only works if you're using simple, single-factor testenvs. It doesn't work when you start taking advantage of combinatorial factors, which is an first class feature of tox. [testenv:py{38,39,310}-django{40,41}]
commands = ... The https://github.com/ymyzk/tox-gh-actions project already exists and supports tox 4 (I'm using it in a few projects of my own, funnily enough). It's far more powerful than simple support for e.g. |
This makes use of tox in CI systems easy since you don't need to convert e.g. '3.8' to '38'. We also take the opportunity to comment the regex and add tests. Signed-off-by: Stephen Finucane <stephen@that.guru>
Added support for period in Python factors, along with interpreter implementation prefixes for all of the interpreters listed in |
Perhaps we can use for start the same thing packaging defines here ttps://github.com/pypa/packaging/blob/63e657138d30063bd46ed16a2e10cfe279a1d7c3/src/packaging/tags.py#L31-L37?
Ideally but I can live with
For this simple use, I'd instead not require users to use a plugin.
This works as long as
We can do that, but I'd add my approach for reasons outlined in my previous paragraph.
Yeah, but those cases are still, in my experience 20%. They can have more complicated CI configs, but for the 80%, I prefer my approach.
I don't like how that project achieves this, and I don't believe we'd need a plugin for this. So I'm soft against it. I reached out to the person over a year ago and they refused, so alas 🦐 , hence why we have the competing https://github.com/tox-dev/tox-gh. But again I'd prefer to not need a plugin for the 80% case. |
Signed-off-by: Bernát Gábor <bgabor8@bloomberg.net>
for more information, see https://pre-commit.ci
3bfadc1
to
89e7404
Compare
Signed-off-by: Bernát Gábor <bgabor8@bloomberg.net>
89e7404
to
fcfe5a4
Compare
🥳 |
This PR contains the following updates: | Package | Type | Update | Change | Pending | |---|---|---|---|---| | [tox](https://togithub.com/tox-dev/tox) ([changelog](https://tox.wiki/en/latest/changelog.html)) | dev | patch | `4.2.6` -> `4.2.8` | `4.4.2` (+8) | --- ### Release Notes <details> <summary>tox-dev/tox</summary> ### [`v4.2.8`](https://togithub.com/tox-dev/tox/releases/tag/4.2.8) [Compare Source](https://togithub.com/tox-dev/tox/compare/4.2.7...4.2.8) #### What's Changed - Allow package names with env markers with pip binary options by [@​q0w](https://togithub.com/q0w) in [https://github.com/tox-dev/tox/pull/2853](https://togithub.com/tox-dev/tox/pull/2853) **Full Changelog**: tox-dev/tox@4.2.7...4.2.8 ### [`v4.2.7`](https://togithub.com/tox-dev/tox/releases/tag/4.2.7) [Compare Source](https://togithub.com/tox-dev/tox/compare/4.2.6...4.2.7) #### What's Changed - Add release notes project URL for quick access in PyPI web by [@​scop](https://togithub.com/scop) in [https://github.com/tox-dev/tox/pull/2835](https://togithub.com/tox-dev/tox/pull/2835) - Document colorization regression on Windows by [@​adamchainz](https://togithub.com/adamchainz) in [https://github.com/tox-dev/tox/pull/2837](https://togithub.com/tox-dev/tox/pull/2837) - The tests actually require wheel by [@​hroncok](https://togithub.com/hroncok) in [https://github.com/tox-dev/tox/pull/2843](https://togithub.com/tox-dev/tox/pull/2843) - Only return Python factor on base_python conflict by [@​stephenfin](https://togithub.com/stephenfin) in [https://github.com/tox-dev/tox/pull/2840](https://togithub.com/tox-dev/tox/pull/2840) - Revert to supporting simple Python factors by [@​stephenfin](https://togithub.com/stephenfin) in [https://github.com/tox-dev/tox/pull/2849](https://togithub.com/tox-dev/tox/pull/2849) **Full Changelog**: tox-dev/tox@4.2.6...4.2.7 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xMDEuMCIsInVwZGF0ZWRJblZlciI6IjM0LjEwMS4wIn0=--> Co-authored-by: descope[bot] <descope[bot]@users.noreply.github.com>
Instead of parsing factors with virtualenv's
PythonSpec.from_string_spec
, use our own regex to support the simple factors that have been supported since tox v1.I have submitted this as a draft since this PR removes the functionality without a deprecation period. tox 4 is only a month old and I've wagered that not many people are relying on this functionality yet. I am, however, happy to rework it so we have a deprecation period instead, though that would require additional changes to fix e.g. #2657 (perhaps a temporary environment option to opt-in to the "new" old behavior?)
Fixes: #2657
Fixes: #2848
Thanks for contribution
Please, make sure you address all the checklists (for details on how see
development documentation)!
tox -e fix
)docs/changelog
folder