Allow nox.parametrize to select the session Python #404
Conversation
| @@ -1,7 +1,7 @@ | |||
| import copy | |||
| import functools | |||
| import types | |||
| from typing import Any, Callable, Dict, Iterable, List, Optional, cast | |||
| from typing import AbstractSet, Any, Callable, Dict, Iterable, List, Optional, cast | |||
There was a problem hiding this comment.
Very minor nitpick: note that AbstractSet is deprecated in 3.9
There was a problem hiding this comment.
Thank you for taking the time to review this.
typing.AbstractSet is superceded by collections.abc.Set in 3.9, but this won't work in earlier Python versions where the abstract collections are not yet generics, i.e. you can't write Set[str]. The same deprecation affects Callable and Iterable; Dict and List are also deprecated in favor of dict and list.
I'm not sure how to address this, do you have an idea? It would be interesting to see how other projects deal with the deprecation. I'm hesitant to introduce a large amount of compatibility boilerplate.
There was a problem hiding this comment.
For what's worth, in pytest the contract when ids are explicitly provided is to receive an Iterable[str] (and to transform it into a list or set internally so as to consume possible generators). Maybe this could be sufficient ? I think that it is the same if you provide argnames as an iterable and not a string containing coma-separated argnames
There was a problem hiding this comment.
The public API does use Iterable[str]:
# _parametrize.py
def parametrize_decorator(
arg_names: Union[str, List[str], Tuple[str]],
arg_values_list: Union[Iterable[ArgValue], ArgValue],
ids: Optional[Iterable[Optional[str]]] = None,
sessionparams: Optional[Iterable[str]] = None,
) -> Callable[[Any], Any]:
...
sessionparams = frozenset(sessionparams if sessionparams is not None else [])
...We only use AbstractSet internally.
There was a problem hiding this comment.
oh sorry I read too fast ! perfect then.
| previous_param_specs = getattr(f, "parametrize", None) | ||
| new_param_specs = update_param_specs(previous_param_specs, param_specs) | ||
| sessionparams |= getattr(f, "sessionparams", frozenset()) |
There was a problem hiding this comment.
Just curious: is this equivalent to
| sessionparams |= getattr(f, "sessionparams", frozenset()) | |
| sessionparams.update(getattr(f, "sessionparams", frozenset())) |
? If so readability might suggest the latter but this is maybe because I am an old pythonista already :)
There was a problem hiding this comment.
Join the club 😉 Yes, the two are almost synonymous, except that update accepts any iterable; both have been around since PEP 218 introduced sets. I'm happy to change this if it's confusing. Personally, I find that using set union and assignment instead of a single mutating function call makes the code more declarative and idiomatic, and avoiding parentheses improves readability.
There was a problem hiding this comment.
I learnt something today thanks ! :) Let's let the core nox team decide then.
|
Only a few nitpicks on my side, otherwise it looks great ! Thanks @cjolowicz |
Fixes #392
Here's a first go at allowing
@nox.parametrizeto select the Python interpreter for a session.