Moving off optparse #4659
Comments
pradyunsg
added
type: enhancement
|
Since optparse is deprecated it would make sense to switch to argparse. |
|
+1 for click from me. I'm pretty sure there'd be some edge case the switch would break but that's fine. |
|
I think you would be better off with argparse as it's in the standard library and you don't have to add a new dependency to it. It also has the argcomplete library which is good for tab completion. Also, read this and see what you think: http://xion.io/post/programming/python-dont-use-click.html |
|
Summary of the blog post: click has an hands-off approach and the author feels that approach isn't appropriate when the CLI is your user interface. That's a fair argument, made in a fairly emotional tone IMO. (maybe that python-dev thread making me look at things this way) I'm happy with both options and likely argparse is going to be easier to switch to anyways. |
pradyunsg
added
state: awaiting PR
|
it should be noted that argparse has a few pretty nasty "bydesign" bugs around automatically aliasing the prefix pf an argument to the full version, |
|
But nevertheless, argparse is the best choice. |
|
@RonnyPfannschmidt do you have any links? My Googling couldn't find anything... :/ |
|
https://docs.python.org/3/library/argparse.html#allow-abbrev - sorry for quoting the python version wrong, it was added in 3.5 |
pradyunsg
added
the
C: cli
|
I'm working on moving to click. Is there anyone else doing it? |
|
I've done a bit of work in the past but ran out of time; mostly was just trying to figure out what all is worth investigating, how things could be done and potential improvements before actually starting working on code. I do have some notes I can share -- none of what's there in the notes is set in stone but might be a good place to start from. You don't need to respond to every hypothetical in it but those should be figured out as we do this. :) Please feel free to discuss what you're thinking here. Investigate:
Plan:
To be used parts of the API:
Avoid direct use:
|
|
Hey @thechief389! Have you made any progress on this? |
|
I had trouble trying to vendor all the libraries and their dependencies for click and click-completion. |
|
Another argument against click. After spending 15 minutes I haven't found the code to implement help in this manner:
There 4. 5. 6. are equivalent to |
|
Hello, I'm one of the Pallets (Click) core maintainers. I'm happy to answer questions you have, although I'd ask that you ping me here or in our Discord rather than opening up issues for questions in the repo. I think we're already set up to be vendorable, and if not we can fix that. If you notice any bugs while trying to integrate Click with Pip, please let us know. |
|
The basic implementation of a @cli.command()
@click.argument("name")
@click.pass_context
def help(ctx, name):
command = cli.get_command(ctx, name)
click.echo(command.get_help(ctx)) |
|
Thanks for pitching in here @davidsm! :) I'll ping here if and when I work on this. :) |
|
Because of the size of the code base, I think it's important to have an implementation plan that would let us make progress on this incrementally. |
|
We can't really have half our parsing be optparse and the other half be click. I think you know that. ;) I think you mean what the plan is for separating concerns within pip's codebase about this to ease the actual "switch". My current thought process for that is:
PS: it's past 1 am. Don't quote me on these. :) |
That's not what I was saying.. I was saying I think it's important for us to have a way to do it that lets us make changes to the code gradually over time. There can be many possible approaches. For example, maybe there is a way to add a translation / compatibility layer to have whatever we're migrating to (argparse, click, or whatever) accept optparse options. That would let us switch the options one at a time from optparse to the new format. The point is to avoid a massive PR or massive amounts of code getting turned on at once. The approach should let us incrementally add new code that is getting used as we add it. |
|
Here's a rough draft of what I'd imagined we'd end up with in terms of how the commands' option declaration looks like. I'm not looking for feedback / inputs on this at this time. I'm noting it here in case someone else comes around to work on this and would like to know a rough target of what I want us to achieve. ...
from pip._internal.cli.base_command import Command
from pip._internal.cli.options import common_options, Choice, IndexOptionsMixin, Option
...
class ListCommand(Command, IndexOptionsMixin):
"""
List installed packages, including editables.
Packages are listed in a case-insensitive sorted order.
"""
usage = """
%prog [options]"""
options = [
Option(
"-o", "--outdated",
is_flag=True,
help="List outdated packages",
),
Option(
"-u", "--uptodate",
is_flag=True,
help="List up-to-date packages",
),
Option(
"-e", "--editable",
is_flag=True,
help="List editable projects.",
),
Option(
"-l",
"--local",
is_flag=True,
help=(
"If in a virtualenv that has global access, do not list "
"globally-installed packages."
),
),
Option(
"--user",
is_flag=True,
help="Only output packages installed in user-site."
),
common_options.list_path,
Option(
"--pre",
is_flag=True,
help=(
"Include pre-release and development versions. By default, "
"pip only finds stable versions."
),
),
Option(
"--format",
type=Choice(["columns", "freeze", "json"]),
default="columns",
dest="list_format",
help=(
"Select the output format among: columns (default), freeze, or json"
),
),
Option(
"--not-required",
is_flag=True,
help="List packages that are not dependencies of installed packages.",
),
Option(
"--include-editable/--exclude-editable",
is_flag=True,
help="Include editable package from output.",
),
]
# No need for wrangling with `self.cmd_opts` in `__init__`
# Rest of the command definition follows immediately. |
|
@pradyunsg Is this still up for development? |
|
@sinscary certainly! |
|
@pradyunsg In that case I would like to give it a try. |
|
Please do! In case you want to have a voice / video chat about this, free free to reach out to me over email. :) |
|
Shameless plug: in case you need additional features for Click, like argparse argument groups, you may consider Cloup. To my knowledge, it's the only package that allows to define option groups without using decorators with Click. |
|
Well, here's a fun thing that's going to make this a bit more painful: I ran 🤦🏽 |
|
How about using sdispater/cleo @pradyunsg? It supports the class-based approach that we are wanting to have, along with some other pretty features like text colouring. How poetry uses it for its CLI - https://github.com/python-poetry/poetry/blob/6485bc23d6497c7731e0f1a635f960b33f2ae99e/src/poetry/console/commands/export.py#L9-L38 |
|
Actually from the above discussion argparse is by far the best choice. The only blocker against it has been removed now pip has dropped all Python <3.6. And since it’s stdlib, nothing can really beat it at this point (plus migrating from optparse to argparse might actually be the least work due to their similarities in interface design). |
|
For argparse, we would need to build some features from scratch as we have currently done for optparse, IMO command-line libraries like click and cleo are doing a much great job at the interface they are providing for building apps and have pretty good features. Migrating to cleo won't be a big pain as they share a bit of similarity, the way we currently have them written is similar to cleo's style. |
|
I wouldn’t mind reviewing a POC if you feel it is easy enough to do. Note that we won’t be using much of the features apart from command line parsing (e.g. colors) in either case since those are already done with other mechanisms. |
|
FWIW it seems that while optparse is "deprecated" it's clearly the opinion, in this discussion about an argparse bug, that optparse will never be removed and argparse development itself has largely slowed down in the face of design issues, |
|
I'd be happy to work on a small POC for this (using |
|
There is, and please feel welcome to do a PoC! |
|
Also I don't know if I'm missing something but in the docs it says, the Hopefully after this PR is merged, we won't need to update it😁! |
|
The doc entry is mostly likely a typo, it’s added much later than the actual implementation. |
For commands like `pip cache` with sub-actions like `remove`, so that e.g. `pip cache re<TAB>` completes to `pip cache remove`. All the existing commands that used such sub-actions followed the same approach for: using a dictionary of names to methods to run, so the implementation is just teaching the `Command` object about this mapping and using it in the autocompletion function. There's no handling for the position of the argument, so e.g. `pip cache re<TAB>` and `pip cache --user re<TAB>` will both complete the final word to `remove`. This is mostly because it was simpler to implement like this, but also I think due to how `optparse` works such invocations are valid, e.g. `pip config --user set global.timeout 60`. Similarly, there's no duplication handling so `pip cache remove re<TAB>` will also complete. This is a feature that may be simpler to implement, or just work out of the box, with some argument parsing libraries, but moving to another such library looks to be quite a bit of work (see discussion[1]). I also took the opportunity to tighten some typing: dropping some use of `Any` Link: pypa#4659 [1] Fixes: pypa#13133
For commands like `pip cache` with sub-actions like `remove`, so that e.g. `pip cache re<TAB>` completes to `pip cache remove`. All the existing commands that used such sub-actions followed the same approach for: using a dictionary of names to methods to run, so the implementation is just teaching the `Command` object about this mapping and using it in the autocompletion function. There's no handling for the position of the argument, so e.g. `pip cache re<TAB>` and `pip cache --user re<TAB>` will both complete the final word to `remove`. This is mostly because it was simpler to implement like this, but also I think due to how `optparse` works such invocations are valid, e.g. `pip config --user set global.timeout 60`. Similarly, there's no duplication handling so `pip cache remove re<TAB>` will also complete. This is a feature that may be simpler to implement, or just work out of the box, with some argument parsing libraries, but moving to another such library looks to be quite a bit of work (see discussion[1]). I also took the opportunity to tighten some typing: dropping some use of `Any` Link: pypa#4659 [1] Fixes: pypa#13133
For commands like `pip cache` with sub-actions like `remove`, so that e.g. `pip cache re<TAB>` completes to `pip cache remove`. All the existing commands that used such sub-actions followed the same approach for: using a dictionary of names to methods to run, so the implementation is just teaching the `Command` object about this mapping and using it in the autocompletion function. There's no handling for the position of the argument, so e.g. `pip cache re<TAB>` and `pip cache --user re<TAB>` will both complete the final word to `remove`. This is mostly because it was simpler to implement like this, but also I think due to how `optparse` works such invocations are valid, e.g. `pip config --user set global.timeout 60`. Similarly, there's no duplication handling so `pip cache remove re<TAB>` will also complete. This is a feature that may be simpler to implement, or just work out of the box, with some argument parsing libraries, but moving to another such library looks to be quite a bit of work (see discussion[1]). I also took the opportunity to tighten some typing: dropping some use of `Any` Link: pypa#4659 [1] Fixes: pypa#13133
For commands like `pip cache` with sub-actions like `remove`, so that e.g. `pip cache re<TAB>` completes to `pip cache remove`. All the existing commands that used such sub-actions followed the same approach for: using a dictionary of names to methods to run, so the implementation is just teaching the `Command` object about this mapping and using it in the autocompletion function. There's no handling for the position of the argument, so e.g. `pip cache re<TAB>` and `pip cache --user re<TAB>` will both complete the final word to `remove`. This is mostly because it was simpler to implement like this, but also I think due to how `optparse` works such invocations are valid, e.g. `pip config --user set global.timeout 60`. Similarly, there's no duplication handling so `pip cache remove re<TAB>` will also complete. This is a feature that may be simpler to implement, or just work out of the box, with some argument parsing libraries, but moving to another such library looks to be quite a bit of work (see discussion[1]). I also took the opportunity to tighten some typing: dropping some use of `Any` Link: pypa#4659 [1] Fixes: pypa#13133
For commands like `pip cache` with sub-commands like `remove`, so that e.g. `pip cache re<TAB>` completes to `pip cache remove`. All the existing commands that used such sub-commands followed the same approach: using a dictionary of names to methods to run, so the implementation is just teaching the `Command` object about this mapping and using it in the autocompletion function. There's no handling for the position of the argument, so e.g. `pip cache re<TAB>` and `pip cache --user re<TAB>` will both complete the final word to `remove`. This is mostly because it was simpler to implement like this, but also I think due to how `optparse` works such invocations are valid, e.g. `pip config --user set global.timeout 60`. This is a feature that may be simpler to implement, or just work out of the box, with some argument parsing libraries, but moving to another such library looks to be quite a bit of work (see discussion[1]). I also took the opportunity to tighten some typing: dropping some use of `Any` Link: pypa#4659 [1] Fixes: pypa#13133
For commands like `pip cache` with sub-commands like `remove`, so that e.g. `pip cache re<TAB>` completes to `pip cache remove`. All the existing commands that used such sub-commands followed the same approach: using a dictionary of names to methods to run, so the implementation is just teaching the `Command` object about this mapping and using it in the autocompletion function. There's no handling for the position of the argument, so e.g. `pip cache re<TAB>` and `pip cache --user re<TAB>` will both complete the final word to `remove`. This is mostly because it was simpler to implement like this, but also I think due to how `optparse` works such invocations are valid, e.g. `pip config --user set global.timeout 60`. This is a feature that may be simpler to implement, or just work out of the box, with some argument parsing libraries, but moving to another such library looks to be quite a bit of work (see discussion[1]). I also took the opportunity to tighten some typing: dropping some use of `Any` Link: pypa#4659 [1] Fixes: pypa#13133
For commands like `pip cache` with sub-commands like `remove`, so that e.g. `pip cache re<TAB>` completes to `pip cache remove`. All the existing commands that used such sub-commands followed the same approach: using a dictionary of names to methods to run, so the implementation is just teaching the `Command` object about this mapping and using it in the autocompletion function. There's no handling for the position of the argument, so e.g. `pip cache re<TAB>` and `pip cache --user re<TAB>` will both complete the final word to `remove`. This is mostly because it was simpler to implement like this, but also I think due to how `optparse` works such invocations are valid, e.g. `pip config --user set global.timeout 60`. This is a feature that may be simpler to implement, or just work out of the box, with some argument parsing libraries, but moving to another such library looks to be quite a bit of work (see discussion[1]). I also took the opportunity to tighten some typing: dropping some use of `Any` Link: pypa#4659 [1] Fixes: pypa#13133
@pypa/pip-committers Thoughts on switching over to using argparse or even click for option parsing?
The text was updated successfully, but these errors were encountered: