feat: support users specifying an undeclared parametrization of python #361
Conversation
|
Can't comment on the code, but hey! Thanks for moving this forward! ^>^ One thing that'd be nice would be being able to say "hey, use the regular |
|
looks okay so far, but I do worry about this being kind of a "hidden"/"implicit" feature. I'm wondering if there's a more explicit way for us to allow amending a python interpreter. |
Would it make sense to support Another method would be a command-line option to globally add an interpreter spec to every requested session. For example, |
|
I do think as is this is rather hidden, and at minimum would require docs to state this is supported behavior. Whilie I like the idea presented by @cjolowicz presents, I think it might be at odds with the error on missing interpreter, which I have seen used to ensure CI is actually testing all scenarios. This sort of makes it fluid what ought to be tested.
|
|
Alright. I decided to move this to the direction suggested by @cjolowicz. TODO: if we use this route changes to usage.rst will be needed to merge. List sessionsList with extra-python argumentRun non existent session with extra pythonRun all unit, with an extra python |
|
What would happen when an existing python is specified via extra-python? |
|
@pradyunsg as authored it will add it to the list of sessions. On extension I could 'set'-ify it to ensure this doesn't happen, but I wasn't particularly concerned with appending a duplicate session. |
Co-authored-by: Claudio Jolowicz <cjolowicz@gmail.com>
|
Incorporated @cjolowicz changes to manage dupes |
Co-authored-by: Claudio Jolowicz <cjolowicz@gmail.com>
|
@cjolowicz are there additional test cases on your mind to guard a bit? Just thinking out loud after the few small tweaks :) |
|
Yes, more tests would be good :) I'm learning to be more careful with the suggest feature 😉 Unfortunately, this last suggestion (169ede8) had a bug, the assertion on the following line can now fail. I think it would be best to revert that commit, and think about how we want to handle the case where
|
|
@cjolowicz I have reverted it locally. If I have a nox session and run However, it is only running the default right now. Before trying to fix that, it may be easier to talk about the expected behavior. When no python is specified in noxfile, but extra-pythons are provided:
I think Other potential issue: Also, when pythons are specified, you can make up session names that don't exist (for instance |
@pradyunsg I think we have this covered now. If you specify |
|
YAY! |
|
Hey @crwilcox
Wouldn't this be FWIW, here's the patch I'm trying this with: diff --git a/nox/manifest.py b/nox/manifest.py
index dca2d0a..7a5085c 100644
--- a/nox/manifest.py
+++ b/nox/manifest.py
@@ -191,13 +191,12 @@ class Manifest:
extra_pythons = self._config.extra_pythons # type: List[str]
if isinstance(func.python, (list, tuple, set)):
func.python = list({*func.python, *extra_pythons})
- elif not multi and func.python:
+ elif not multi and func.python is not False:
# if this is multi, but there is only a single interpreter, it
# is the reentrant case. The extra_python interpreter shouldn't
# be added in that case. If func.python is False, the session
- # has no backend; treat None and the empty string equivalently.
- # Otherwise, add the extra specified python.
- assert isinstance(func.python, str)
+ # has no backend. Otherwise, add the extra specified python.
+ assert func.python is None or isinstance(func.python, str)
func.python = list({func.python, *extra_pythons})
# If the func has the python attribute set to a list, we'll need
I see your point here. I'll try to repeat this in my own words: If a user did not specify a Python version for a session, that should not prevent them from running the session on some specific interpreter via There are some things that bother me about this behavior, though:
Edit: It doesn't. I think the general point still holds, but it turns out that the
On the other hand, I think the use case of running a session without a Python version on some specific interpreter is quite valid. So I guess I'm leaning towards your option 2 ("Run only extra-pythons"). I'd see this as an override, replacing I think the difference in behavior compared to sessions with a That could be done with the following patch: diff --git a/nox/manifest.py b/nox/manifest.py
index dca2d0a..66a120c 100644
--- a/nox/manifest.py
+++ b/nox/manifest.py
@@ -191,14 +191,16 @@ class Manifest:
extra_pythons = self._config.extra_pythons # type: List[str]
if isinstance(func.python, (list, tuple, set)):
func.python = list({*func.python, *extra_pythons})
- elif not multi and func.python:
+ elif not multi:
# if this is multi, but there is only a single interpreter, it
# is the reentrant case. The extra_python interpreter shouldn't
# be added in that case. If func.python is False, the session
- # has no backend; treat None and the empty string equivalently.
- # Otherwise, add the extra specified python.
- assert isinstance(func.python, str)
- func.python = list({func.python, *extra_pythons})
+ # has no backend. Otherwise, add the extra specified python.
+ if func.python is None:
+ func.python = extra_pythons[:]
+ elif func.python:
+ assert isinstance(func.python, str)
+ func.python = list({func.python, *extra_pythons})
# If the func has the python attribute set to a list, we'll need
# to expand them.Using this patch on your example: $ nox --extra-python=3.10 -l
Sessions defined in /tmp/nox-361/noxfile.py:
* unit_no_python-3.10 -> Run the unit test suite.
sessions marked with * are selected, sessions marked with - are skipped.```
$ nox --extra-python=3.9 --session=unit_no_python
nox > Running session unit_no_python-3.9
nox > Creating virtual environment (virtualenv) using python3.9 in .nox/unit_no_python-3-9
nox > Session unit_no_python-3.9 was successful. |
|
There is another issue with the current implementation. Sets do not preserve order, so the Python interpreters are run in an unpredictable order for each session. We can fix this using diff --git a/nox/manifest.py b/nox/manifest.py
index 66a120c..930e490 100644
--- a/nox/manifest.py
+++ b/nox/manifest.py
@@ -15,6 +15,7 @@
import argparse
import collections.abc
import itertools
+from collections import OrderedDict
from typing import Any, Iterable, Iterator, List, Mapping, Sequence, Set, Tuple, Union
from nox._decorators import Call, Func
@@ -23,6 +24,11 @@ from nox.sessions import Session, SessionRunner
WARN_PYTHONS_IGNORED = "python_ignored"
+def _unique_list(*args: str) -> List[str]:
+ """Return a list without duplicates, while preserving order."""
+ return list(OrderedDict.fromkeys(args))
+
+
class Manifest:
"""Session manifest.
@@ -190,7 +196,7 @@ class Manifest:
# include additional python interpreters
extra_pythons = self._config.extra_pythons # type: List[str]
if isinstance(func.python, (list, tuple, set)):
- func.python = list({*func.python, *extra_pythons})
+ func.python = _unique_list(*func.python, *extra_pythons)
elif not multi:
# if this is multi, but there is only a single interpreter, it
# is the reentrant case. The extra_python interpreter shouldn't
@@ -200,7 +206,7 @@ class Manifest:
func.python = extra_pythons[:]
elif func.python:
assert isinstance(func.python, str)
- func.python = list({func.python, *extra_pythons})
+ func.python = _unique_list(func.python, *extra_pythons)
# If the func has the python attribute set to a list, we'll need
# to expand them. |
The code in this branch returns the nox python if no python is specified, not none, at least at this point of nox. The data I was reporting was based off observed state, sometimes via debugger to inspect along the way. So I don't believe there is a time we have a sequence that mixes
I think this is a reasonable thought. I feel like having extra-pythons override in the case no python is specified is reasonable.
Thanks for bringing this up. It sort of bothered me also but wasn't prioritizing fixing that up. But now that I am not the only one irritated I am going to knock that off the list of TODOs :) |
Co-authored-by: Danny Hermes <daniel.j.hermes@gmail.com>
|
@pradyunsg checking in since this has stabilized, does the support here do what is needed for pip? test case at https://github.com/theacodes/nox/pull/361/files#diff-03dd41d84b05bb2477b17b322a4b5b1f3de5ccc6bc9957c09d46f87849eb3a26R207 gives a good idea of the complete behavior |
|
That'll work. As long as I can test for 3.10 / PyPy x.x.x without modifying the noxfile, I'm a happy kiddo. :) How friendly is this toward non-CPython implementations being the runner, like PyPy, Ironpython and whatnot? |
|
This is the equivalent of specifying the string in python = [""] in the session attribute. So those should be doable. |
|
Awesome. I installed1 and played around with this PR today, and this works great! The only question/concern I have now is: I don't see any way to only run a session with something from I can imagine wanting to run with the unqualified version (this is especially relevant with like, pip's test session which someone might only wanna run on one interpreter, once). Other than that, I don't have too many thoughts. 1 I quite enjoy being able to do |
Hi @pradyunsg would That should run the session only with python from PATH, which could be a pypy installation. For example, I just tried this with the following noxfile: import nox
@nox.session(python="3.9")
def test(session):
session.run("python", "-VV")Below I use pypy as the default Python (via pyenv). By default, the nox session uses CPython 3.9. Passing $ pyenv shell pypy3.6-7.3.0 3.9.0
$ python -VV
Python 3.6.9 (1608da62bfc7, Dec 23 2019, 10:50:17)
[PyPy 7.3.0 with GCC 4.2.1 Compatible Apple LLVM 10.0.0 (clang-1000.11.45.5)]
$ python -m venv venv
$ . venv/bin/activate
$ pip install https://github.com/crwilcox/nox/archive/allow-non-specified-python-versions.zip
$ nox -s test
nox > Running session test
nox > Creating virtual environment (virtualenv) using python3.9 in .nox/test
nox > python -VV
Python 3.9.0 (default, Oct 5 2020, 22:21:26)
[Clang 11.0.0 (clang-1100.0.20.17)]
nox > Session test was successful.
$ nox -s test --extra-python python --python python
nox > Running session test-python
nox > Creating virtual environment (virtualenv) using python in .nox/test-python
nox > python -VV
Python 3.6.9 (1608da62bfc7, Dec 23 2019, 10:50:17)
[PyPy 7.3.0 with GCC 4.2.1 Compatible Apple LLVM 10.0.0 (clang-1000.11.45.5)]
nox > Session test-python was successful. |
|
You can also run a specific session and add extra pythons which would only run the extra pythons |
|
AHA! That does indeed work @cjolowicz! I imagine the documentation would doing the heavy lifting of explaining this, because that wasn't immediately obvious to me (and I didn't look at the docs on this PR, sorry!). |
I've been wondering if the combination The other non-obvious thing here, I guess, would be the use of |
Yes please. (i really don't have more to say, oops) |
|
@cjolowicz do you want the doc changes as part of this pr or should we spin that as a new workitem? |
|
@crwilcox I think actually all this needs is something at the end of the section about This option can be combined with the ``--python`` option
to change the Python interpreter for a session,
replacing any versions listed in the configuration file::
nox --extra-python 3.10 --python 3.10 -s lint
You can also specify ``python`` instead of a version.
This will run sessions using whichever ``python`` command you have on your ``PATH``::
nox --extra-python python --python pythonIn that case, we won't be touching unrelated documentation sections, so it feels like there's no need for a separate PR. Documenting |
|
@cjolowicz let me know what you think of the docs add. I think putting it under the additional python header makes good sense. |
Co-authored-by: Claudio Jolowicz <cjolowicz@gmail.com>
Co-authored-by: Claudio Jolowicz <cjolowicz@gmail.com>
|
Oh, good catch! Thanks! If no objections, I will merge this later today/tomorrow. I think this is in a good place 👍 |
| ("3.5", ["3.8", "3.9"], ["3.5", "3.8", "3.9"]), | ||
| (["3.5", "3.9"], [], ["3.5", "3.9"]), | ||
| (["3.5", "3.9"], ["3.8"], ["3.5", "3.9", "3.8"]), | ||
| (["3.5", "3.9"], ["3.8", "3.4"], ["3.5", "3.9", "3.8", "3.4"]), |
There was a problem hiding this comment.
Should be good to have a case where an interpreter in extra_pythons is already in python (to test the _unique_list function).
|
Hurray! Thanks for doing this folks! ^.^ |
Closes #158
This adds coverage for
--extra-pythonwhich allows for adding additional pythons for running python interpreters, not specified in the noxfile.