thebjorn/pydeps

Variadic optional arguments break fname parsing

Closed this issue · 3 comments

glumia commented

Hi!

When one tries to use an option that takes a variable number of arguments (eg. --only) at the end of the command, pydeps complains about fname not being provided. This is caused by the arg parser taking fname as an argument of the previous option.

Some examples (commands executed inside pydeps main directory) :

$ pydeps --only pydeps pydeps
usage: pydeps [-h] [--debug] [--config FILE] [--no-config] [--version]
              [-L LOG] [--find-package] [-v] [-o file] [-T FORMAT]
              [--display PROGRAM] [--noshow] [--show-deps] [--show-raw-deps]
              [--show-dot] [--nodot] [--no-output] [--show-cycles]
              [--debug-mf INT] [--noise-level INT] [--max-bacon INT] [--pylib]
              [--pylib-all] [--include-missing] [-x PATTERN [PATTERN ...]]
              [-xx MODULE [MODULE ...]] [--only MODULE_PATH [MODULE_PATH ...]]
              [--externals] [--reverse] [--cluster] [--min-cluster-size INT]
              [--max-cluster-size INT] [--keep-target-cluster]
              [--rmprefix PREFIX [PREFIX ...]] [--start-color INT]
              fname
pydeps: error: the following arguments are required: fname
$ pydeps -x os sys pydeps
usage: pydeps [-h] [--debug] [--config FILE] [--no-config] [--version]
              [-L LOG] [--find-package] [-v] [-o file] [-T FORMAT]
              [--display PROGRAM] [--noshow] [--show-deps] [--show-raw-deps]
              [--show-dot] [--nodot] [--no-output] [--show-cycles]
              [--debug-mf INT] [--noise-level INT] [--max-bacon INT] [--pylib]
              [--pylib-all] [--include-missing] [-x PATTERN [PATTERN ...]]
              [-xx MODULE [MODULE ...]] [--only MODULE_PATH [MODULE_PATH ...]]
              [--externals] [--reverse] [--cluster] [--min-cluster-size INT]
              [--max-cluster-size INT] [--keep-target-cluster]
              [--rmprefix PREFIX [PREFIX ...]] [--start-color INT]
              fname
pydeps: error: the following arguments are required: fname
$ pydeps --only pydeps --show-deps pydeps # This works
...
$ pydeps -x os sys --show-deps  pydeps # This works
...
thebjorn commented

Yes, there is no way for argparse to know when the variable length options end (unless another option is given).

You can work around this by moving the variable length options to the end (the following versions of your first two examples will work):

pydeps pydeps --only pydeps
pydeps pydeps -x os sys

I'm open to suggestions for how to make this clearer in the docs...

  • 👍1
glumia commented

Yeah, I tought at first it was a bug in pydeps' code but then discovered it's rooted in argparse and it's why something like -- exists in bash.

I would add a note on this in the Usage (parameters) section of the readme since some people could wrongly expect that pydeps reserves the last argument for fname (like I did).

Eg.:

**Note:** if an option with a variable number of arguments (like `-x`) is provided before `fname` separe the arguments from the filename with `--`, otherwise `fname` will be parsed as an argument of the option.   
Example: `$ pydeps -x os sys -- pydeps`.

I would also suggest to change the order of the readme contents in:

  • How to install
  • Basic Usage
  • Usage
  • Usage (parameters)
  • .pydeps
  • Bacon (Scoring)
  • Import cycles
  • Clustering externals
  • Intermediate format
  • Version history (this could be moved to a separate CHANGELOG.md file)
  • Contributing

In this way a user would see immediatly how pydeps should be used.

@thebjorn what do you think about it? I can open a pull request with those changes if it's okay for you :)

thebjorn commented

That sounds good to me. It's a great help to have new eyes on these things :-)
The -- solution is handled by argparse and also works on windows.

I think I prefer to keep the version history in the readme since it lists all contributors (pydeps wouldn't be the same without all the great contributions I've gotten).

I'll happily merge a PR.

  • 👍1