Skip to content

Need option for not skipping missing interpreters #631

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

Closed
wsanchez opened this issue Sep 13, 2017 · 15 comments
Closed

Need option for not skipping missing interpreters #631

wsanchez opened this issue Sep 13, 2017 · 15 comments
Labels
area:configuration feature:change something exists already but should behave differently needs:discussion It's not quite clear if and how this should be done

Comments

@wsanchez
Copy link

I really like to have skip_missing_interpreters = True in my tox.ini, because I'm often working on machines that don't have every Python interpreter installed.

But it's a really bad idea in a CI scenario, because environments just pass without doing anything.

I'd love to see an option to set that value back to False, but there's only a CLI option for setting it to True… every flag really should have an opposite.

@obestwalter obestwalter added area:configuration feature:change something exists already but should behave differently labels Sep 14, 2017
@obestwalter
Copy link
Member

obestwalter commented Sep 14, 2017

Hi @wsanchez, you could make the value dependent on an env var and get what you want this way.

[tox]
skip_missing_interpreters = {env:TOX_SKIP_MISSING_INTERPRETERS:False}

On your devbox have the env var permanently set by adding it to the initialization of the shell or directly on the command line:

$ TOX_SKIP_MISSING_INTERPRETERS=True tox  #posix

$ set TOX_SKIP_MISSING_INTERPRETERS=True & tox  #windows

CI can then be blissfully unaware of that configuration option and developers can adjust it to their needs.

@asottile
Copy link
Contributor

I for one think this is a good idea, patch incoming :)

@obestwalter
Copy link
Member

I am -1 on this. Feels like command line argument inflation for something that is easily solvable with what we have.

@obestwalter
Copy link
Member

I would rather introduce accepting an argument for --skip-missing-interpreters that behaves exactly like the configuration option. That would be a breaking change, but much cleaner.

@obestwalter obestwalter added the needs:discussion It's not quite clear if and how this should be done label Sep 14, 2017
@asottile
Copy link
Contributor

Devils advocate: why even have the command line argument if you could do it with environment variables 😆

@asottile
Copy link
Contributor

more seriously: I think we could change --skip-missing-interpreters to take an argument and preserve the existing behaviour if no argument is passed -- if that's a better option :)

import argparse
import distutils.util

no_arg = object()


parser = argparse.ArgumentParser()
parser.add_argument(
    '--foo', default=no_arg, nargs='?', type=distutils.util.strtobool,
)
args = parser.parse_args()

if args.foo is no_arg:
    print('not present: false')
elif args.foo is None:
    print('old behaviour: true')
else:
    print(f'new behaviour {args.foo}')
$ python3.6 test.py
not present: false
$ python3.6 test.py --foo
old behaviour: true
$ python3.6 test.py --foo=True
new behaviour 1
$ python3.6 test.py --foo=False
new behaviour 0
$ python3.6 test.py --foo=garbage
usage: test.py [-h] [--foo [FOO]]
test.py: error: argument --foo: invalid strtobool value: 'garbage'

@obestwalter
Copy link
Member

Now we're getting somewhere :)

@The-Compiler
Copy link
Member

The-Compiler commented Sep 14, 2017

That will break when there are arguments following:

$ python3.6 t.py --foo test.py
usage: t.py [-h] [--foo [FOO]]
t.py: error: argument --foo: invalid strtobool value: 'test.py'

@obestwalter
Copy link
Member

TBH my feeling is anyway to try to simplify tox configuration instead of piling the features on, so I will stay with my -1 on this.

@The-Compiler
Copy link
Member

The-Compiler commented Sep 14, 2017

Maybe tox should just have an -o option to override any ini option, like pytest has? With that, you could do tox -o skip_missing_interpreters=false but also e.g. tox -o distdir=foo, and maybe even tox -o '[testenv:py35]basepython=python3.5'.

@obestwalter
Copy link
Member

That sounds like a great direction to me.

@asottile
Copy link
Contributor

asottile commented Sep 14, 2017

Ooh that sounds like the best option.

If we wanted to make it generic perhaps something like this?

  • -o tox.skip_missing_interpreters=True
  • --option testenv:py35.basepython=python3.5

Or with brackets?

  • -o [tox]skip_missing_interpreters=True
  • -o [testenv:py35]basepython=True

EDIT: that argparse snippet above only fails on positional arguments afterwards -- it still does the right thing on options:

$ python3.6 test.py --foo --bar
usage: test.py [-h] [--foo [FOO]]
test.py: error: unrecognized arguments: --bar

@obestwalter
Copy link
Member

I'd say we have good look at how pytest does this and ideally implement the exact same behaviour. I'm off to bed for now though. Wrestling with that horrible release workflow I concocted left me utterly drained.

@The-Compiler
Copy link
Member

@asottile The syntax with brackets I proposed was inspired by the interpolation syntax in tox.ini where you'd use {[testenv:py35]basepython} as well, so I think it'd make sense to keep that syntax.

EDIT: that argparse snippet above only fails on positional arguments afterwards -- it still does the right thing on options:

Right. It still means invocations which worked before will give a confusing error afterwards.

I'd say we have good look at how pytest does this and ideally implement the exact same behaviour.

Note that pytest doesn't have to handle different config sections, it only has [pytest].

@obestwalter
Copy link
Member

closing in favour of #634 (and because it is possible without this option already as dexcribed in #631 (comment)).

@tox-dev tox-dev locked and limited conversation to collaborators Jan 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area:configuration feature:change something exists already but should behave differently needs:discussion It's not quite clear if and how this should be done
Projects
None yet
Development

No branches or pull requests

4 participants