Skip to content
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

drop argparse since its broken by design and unfixable #1149

Closed
RonnyPfannschmidt opened this issue Oct 21, 2015 · 13 comments

Comments

Projects
None yet
7 participants
@RonnyPfannschmidt
Copy link
Member

commented Oct 21, 2015

relates to #1146

argparse has a prefix-matching mode that will invariably break user code in the preparse step unless they pick option names that cannot share a prefix with arguments known in preparse

@hpk42

This comment has been minimized.

Copy link
Contributor

commented Oct 29, 2015

why do we need to avoid prefix matching, can you give an example?
And if we need to avoid it, why can we not hack argparse?

FWIW I don't think we should drop argparse easily.

@RonnyPfannschmidt

This comment has been minimized.

Copy link
Member Author

commented Oct 29, 2015

Preparse will prefix match on not yet defined plugin/contest options, and it can't be avoided by design, see the linked issue

@The-Compiler

This comment has been minimized.

Copy link
Member

commented Feb 10, 2016

FWIW argparse has allow_abbrev=False with Python 3.5 - maybe it's possible to backport this without too much pain? Commit: https://hg.python.org/cpython/rev/99302634d756

@RonnyPfannschmidt

This comment has been minimized.

Copy link
Member Author

commented Feb 10, 2016

if the only way to correctly work with stdlib modules is to work with backports in some manner, then please lets stop to depend on the stdlib

@The-Compiler

This comment has been minimized.

Copy link
Member

commented Feb 10, 2016

And break every plugin and testsuite using custom arguments? I'd rather not do that, even in a major release 😉

I think it could be as easy as subclassing and overriding _get_option_tuples to always return an empty list, and that'd be it. And I think the risk is manageable as it's only for old Python versions which probably won't change anymore.

(But of course this needs further testing - I only looked at the 3.4 code, and didn't actually try this approach)

@nicoddemus

This comment has been minimized.

Copy link
Member

commented Nov 23, 2016

Just to give an example of the problem @RonnyPfannschmidt mentions, in #2077 the user tried to define an option named --conf in a conftest file, but argparse will always match --confcutdir instead because that's done in the preparse stage to obtain the "known_args".

That's a difficult problem to solve, because we need to pre-parse the argument list to get some options that are important for the discovery of conftest files which in turn might register options that match the which were already parsed.

It seems to me the only real solution would be to remove option prefix matching altogether.

@RonnyPfannschmidt

This comment has been minimized.

Copy link
Member Author

commented Nov 23, 2016

@nicoddemus the prefix matching cannot be disabled until python 3.6

@nicoddemus

This comment has been minimized.

Copy link
Member

commented Nov 23, 2016

You mean 3.5 right? Following what @The-Compiler said:

about FWIW argparse has allow_abbrev=False with Python 3.5

which seems to be the case.

Anyway, perhaps we can hack that into our MyOptionParser subclass...

@nicoddemus

This comment has been minimized.

Copy link
Member

commented Nov 23, 2016

I mean, we don't need to backport the entire patch, which is more complete because it makes prefix matching optional; we only need to disable it altogether.

@DuncanBetts

This comment has been minimized.

Copy link
Contributor

commented Nov 29, 2016

It sounds like backporting parts of the patch would resolve this issue in the most backwards compatible way. If you are seeking an alternative to argparse, perhaps docopt would be a good choice. Thought it was worth a mention anyway :).

@RonnyPfannschmidt

This comment has been minimized.

Copy link
Member Author

commented Nov 29, 2016

@DuncanBetts docopts it no option at all, since it works completely contrary to any api pytest currently has,
as such its a backward incompatible change we cannot impose

@blueyed

This comment has been minimized.

Copy link
Contributor

commented Apr 20, 2018

I also think that we should try to keep argparse, but patch it (temporarily) to make it work.
See #3413 (comment) for some early investigation.

@Zac-HD

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

Closed by #5469.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.