cucapra / pollen

generating hardware accelerators for pangenomic graph queries
MIT License
24 stars 1 forks source link

slow-odgi: CLI #68

Closed anshumanmohan closed 1 year ago

anshumanmohan commented 1 year ago

This PR adds a simple CLI to slow-odgi. The installation instructions are in the README. The testing suite has been brought up to speed, meaning that make test-slow-odgi works as it used to.

anshumanmohan commented 1 year ago

Running python3 -m slow_odgi -h gives:

usage: __main__.py [-h] [GRAPH] COMMAND ...

positional arguments:
  GRAPH       Input GFA file

optional arguments:
  -h, --help  show this help message and exit

slow-odgi commands:
  COMMAND
    chop      Shortens segments' sequences to a given maximum length.
    crush     Replaces consecutive instances of `N` with a single `N`.
    degree    Generates a table summarizing each segment's degree.
    (... elided by Anshuman)

Quick thing: I don't love the ellipsis after COMMAND; it makes it look like many commands are possible. The parser itself (correctly) balks if you try it:

$ python3 -m slow_odgi test/k.gfa paths crush
usage: __main__.py [-h] [GRAPH] COMMAND ...
__main__.py: error: unrecognized arguments: crush
anshumanmohan commented 1 year ago

Now a more serious question: some commands require additional inputs, be it a number or the name of a supplemental file. I've gotten the parser to require these appropriately, but the "help" strings for those command-specific requirements don't render nicely (read, at all) and so the user feels stupid. Thoughts on rendering that in a nicer way?

For example, after seeing the help message I've shown above, the user may try:

$ python3 -m slow_odgi test/k.gfa chop
usage: __main__.py [GRAPH] chop [-h] -n [N]
__main__.py [GRAPH] chop: error: the following arguments are required: -n

or

$ python3 -m slow_odgi test/k.gfa overlap
usage: __main__.py [GRAPH] overlap [-h] -paths [PATHS]
__main__.py [GRAPH] overlap: error: the following arguments are required: -paths

and be frustrated.

sampsyo commented 1 year ago

Awesome! More complete review forthcoming, but for now:

I've gotten the parser to require these appropriately, but the "help" strings for those command-specific requirements don't render nicely (read, ever) and so the user feels stupid. Thoughts on rendering that in a nicer way?

Does python3 -m slow_odgi test/k.gfa chop --help work?

sampsyo commented 1 year ago

That's weird—I just tried it here on this branch and it worked as expected!

$ python3 -m slow_odgi test/k.gfa chop --help
usage: __main__.py [GRAPH] chop [-h] -n [N]

options:
  -h, --help  show this help message and exit
  -n [N]      The max segment size desired after chopping.

Same with -h. Not sure how to explain that…

anshumanmohan commented 1 year ago

I've deleted/hidden some comments to make this thread more readable.

anshumanmohan commented 1 year ago

The latest commit changes the order of arguments so that the command is specified, as we wanted, before the graph. (The quick and dirty thing I did in your office was a no-go; you were right in that we needed to add the graph arg to each subparser.)

The top-level help now looks like

$ python3 -m slow_odgi -h
usage: __main__.py [-h] COMMAND ...

optional arguments:
  -h, --help  show this help message and exit

slow-odgi commands:
  COMMAND
    chop      Shortens segments' sequences to a given maximum length.
    crush     Replaces consecutive instances of `N` with a single `N`.
    ... elided by me

and the command-specific help explains that you need a graph and possibly other stuff.

$ python3 -m slow_odgi chop -h
usage: __main__.py chop [-h] -n [N] [GRAPH]

positional arguments:
  GRAPH       Input GFA file

optional arguments:
  -h, --help  show this help message and exit
  -n [N]      The max segment size desired after chopping.

This all parses out fine, and from a little stackoverflow sleuthing it looks like it's a reasonable way to go. I guess I would have preferred a more illustrative top-level help, with insights right there about what the various subcommands need. But ah well, I'm happy to move on if this is fine!

anshumanmohan commented 1 year ago

Just in the interest of completeness, my vision for the top-level help was:

$ python3 -m slow_odgi -h
usage: __main__.py [-h] COMMAND ... GRAPH

optional arguments:
  -h, --help  show this help message and exit

positional arguments:
  GRAPH       Input GFA file

slow-odgi commands:
  COMMAND
    chop      Shortens segments' sequences to a given maximum length.
              -n The max segment size desired after chopping.
    crush     Replaces consecutive instances of `N` with a single `N`.
    ... elided by me
anshumanmohan commented 1 year ago

flit support is in! I haven't yet published it to the PyPI, but you can install by navigating to ...pollen/slow_odgi and then running flit install --user --symlink.

Then slow_odgi -h will give you the help page as before.

anshumanmohan commented 1 year ago

Testing is back; make test-slow-odgi works again.

sampsyo commented 1 year ago

Great! FWIW, I think rolling with the help as argparse wants it is the way to go… aside from not fighting the library and just doing what it wants to do, this output is nice for a couple of other reasons: