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

Implement improved & consistent argument parsing #785

Merged
merged 34 commits into from Jan 8, 2020
Merged

Implement improved & consistent argument parsing #785

merged 34 commits into from Jan 8, 2020

Conversation

@standage
Copy link
Member

standage commented Dec 5, 2019

The sourmash CLI is a bit hairy using a combination of approaches to parse subcommand and subsubcommand arguments. This is my first pass at implementing a consistent approach using vanilla argparse. A lot of copy/paste is still required, and some tests will need to be updated, but the core design I propose is well demonstrated already.

UPDATE: The proposed CLI implementation is complete—that is, --help works for all commands and subcommands. Having the CLI invoke the appropriate methods is pending. Here I propose making a new subcommand group (e.g. sourmash sbt combine instead of sourmash sbt_combine, sourmash sbt categorize instead of sourmash categorize, and so on). This is also implemented, along with a few dirty hacks to support the old CLI (dirty hacks are annotated to simplify removal on a major version bump).

Closes #783.

  • Is it mergeable?
  • make test Did it pass the tests?
  • make coverage Is the new code covered?
  • Did it change the command-line interface? Only additions are allowed
    without a major version increment. Changing file formats also requires a
    major version number increment.
  • Was a spellchecker run on the source code and documentation after
    changes were made?
cli-sandbox/README.md Outdated Show resolved Hide resolved
@codecov

This comment has been minimized.

Copy link

codecov bot commented Dec 5, 2019

Codecov Report

Merging #785 into master will increase coverage by 0.14%.
The diff coverage is 97.46%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #785      +/-   ##
==========================================
+ Coverage   79.21%   79.36%   +0.14%     
==========================================
  Files          45       83      +38     
  Lines        6707     7017     +310     
  Branches      469      469              
==========================================
+ Hits         5313     5569     +256     
- Misses       1094     1148      +54     
  Partials      300      300
Flag Coverage Δ
#pytests 89.95% <97.46%> (-0.48%) ⬇️
#rusttests 48.71% <ø> (ø) ⬆️
Impacted Files Coverage Δ
sourmash/lca/command_summarize.py 80.24% <ø> (-2.55%) ⬇️
sourmash/commands.py 83.69% <ø> (-4.26%) ⬇️
sourmash/lca/command_rankinfo.py 89.13% <ø> (-1.62%) ⬇️
sourmash/lca/command_compare_csv.py 92.3% <ø> (-1.12%) ⬇️
sourmash/sourmash_args.py 96.03% <ø> (-0.7%) ⬇️
sourmash/sbt.py 86.61% <ø> (ø) ⬆️
sourmash/lca/command_classify.py 84.21% <ø> (-2.16%) ⬇️
sourmash/lca/command_gather.py 83.76% <ø> (-1.09%) ⬇️
sourmash/__main__.py 92.3% <100%> (-3.7%) ⬇️
sourmash/cli/lca/classify.py 100% <100%> (ø)
... and 82 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6a2a14e...219429d. Read the comment docs.

@ctb

This comment has been minimized.

Copy link
Member

ctb commented Dec 6, 2019

@standage

This comment has been minimized.

Copy link
Member Author

standage commented Dec 6, 2019

(do you want contributions to this PR, if I feel so motivated? :)

I'm not expecting any this early, but any feedback or contributions would be welcome. Pointers and contributions would certainly be welcome as the CLI matures and we need to start updating the test suite to compensate.

@standage

This comment has been minimized.

Copy link
Member Author

standage commented Dec 6, 2019

Ok, the CLI is fully implemented—as in sourmash <cmd> --help and sourmash <cmd> <subcmd> --help displays the correct options for all commands and subcommands. The only (sub)commands that are fully functional at the moment are sourmash info and sourmash signature describe. But hopefully this is enough to show what I have in mind.

$ sourmash
usage: sourmash [-h] [-v] [-q] cmd ...

Compute, compare, manipulate, and analyze MinHash sketches of DNA sequences.

Commands:
  Invoke "sourmash <cmd> --help" for more details on executing each command.

  cmd            compare -- compute -- gather -- info -- lca -- plot -- sbt --
                 search -- signature

Options:
  -h, --help     show this help message and exit
  -v, --version  show program's version number and exit
  -q, --quiet    don't print citation information
$
$ sourmash info --help
usage: sourmash info [-h] [--verbose]

optional arguments:
  -h, --help  show this help message and exit
  --verbose   report versions of khmer and screed
$
$ sourmash info --verbose
== This is sourmash version 2.3.1.dev2+ge416b98. ==
== Please cite Brown and Irber (2016), doi:10.21105/joss.00027. ==

sourmash version 2.3.1.dev2+ge416b98
- loaded from path: /Users/standage/Projects/sourmash/sourmash/cli

khmer version 3.0.0a3
- loaded from path: /Users/standage/anaconda3/envs/sourmashdev/lib/python3.7/site-packages/khmer

screed version 1.0.1
- loaded from path: /Users/standage/anaconda3/envs/sourmashdev/lib/python3.7/site-packages/screed
$
$ sourmash signature --help
usage: sourmash signature [-h] subcmd ...

Subcommands:
  Invoke "sourmash signature <subcmd> --help" for more details on executing
  each subcommand.

  subcmd      describe -- downsample -- export -- extract -- flatten -- ingest
              -- intersect -- merge -- overlap -- rename -- subtract

Options:
  -h, --help  show this help message and exit
$
$ sourmash signature describe --help
usage: sourmash signature describe [-h] [-q] [--csv FILE]
                                   signatures [signatures ...]

positional arguments:
  signatures

optional arguments:
  -h, --help   show this help message and exit
  -q, --quiet  suppress non-error output
  --csv FILE   output information to a CSV file
$
$ sourmash signature describe tests/test-data/2.fa.sig
== This is sourmash version 2.3.1.dev2+ge416b98. ==
== Please cite Brown and Irber (2016), doi:10.21105/joss.00027. ==

loaded 3 signatures total.
---
signature filename: tests/test-data/2.fa.sig
signature: CP001071.1 Akkermansia muciniphila ATCC BAA-835, complete genome
source file: 2.fa
md5: f372e47893edd349e5956f8b0d8dcbf7
k=21 molecule=DNA num=0 scaled=1000 seed=42 track_abundance=0
size: 2608
signature license: CC0

---
signature filename: tests/test-data/2.fa.sig
signature: CP001071.1 Akkermansia muciniphila ATCC BAA-835, complete genome
source file: 2.fa
md5: f3a90d4e5528864a5bcc8434b0d0c3b1
k=31 molecule=DNA num=0 scaled=1000 seed=42 track_abundance=0
size: 2701
signature license: CC0

---
signature filename: tests/test-data/2.fa.sig
signature: CP001071.1 Akkermansia muciniphila ATCC BAA-835, complete genome
source file: 2.fa
md5: 43f3b48e59443092850964d355a20ac0
k=51 molecule=DNA num=0 scaled=1000 seed=42 track_abundance=0
size: 2585
signature license: CC0

Design

Each command/subcommand will have a:

  • subparser function for defining its command line interface
  • a driver function (with a name that matches the command/subcommand) with the implementation
  • a main function acting as a thin wrapper around the driver function; facilitates automated testing using simulate command-line arguments
@standage

This comment has been minimized.

Copy link
Member Author

standage commented Dec 6, 2019

Part of the redesign is to have the SBT subcommands grouped under the sourmash sbt command: sourmash sbt merge, sourmash sbt watch, and so on. Of course, we can't break the API without a major version bump, so I added some dirty hacks to support both the old and new API simultaneously. Each instance is marked as "dirty hack" with a note that, if desired, it can be removed with a major version bump.

Incidentally, sourmash signature import was difficult to implement using my strategy since modules cannot be named import. So I used a similar dirty hack to make that work as well.

# BEGIN: dirty hacks to simultaneously support new and previous interface
if hasattr(args, 'subcmd') and args.subcmd == 'import':
args.subcmd = 'ingest'
if hasattr(args, 'cmd') and args.cmd == 'sbt_combine':
args.cmd = 'sbt'
args.subcmd = 'combine'
if hasattr(args, 'cmd') and args.cmd == 'index':
args.cmd = 'sbt'
args.subcmd = 'index'
if hasattr(args, 'cmd') and args.cmd == 'categorize':
args.cmd = 'sbt'
args.subcmd = 'categorize'
if hasattr(args, 'cmd') and args.cmd == 'watch':
args.subcmd = 'sbt'
args.subcmd = 'watch'
if hasattr(args, 'subcmd') and args.subcmd == 'compare_csv':
args.subcmd = 'compare'
# END: dirty hacks to simultaneously support new and previous interface
Comment on lines 47 to 87

This comment has been minimized.

Copy link
@standage

standage Dec 6, 2019

Author Member

Part of the dirty hacks mentioned

# BEGIN: dirty hacks to simultaneously support new and previous interface
sbt.categorize.subparser(sub)
sbt.combine.alt_subparser(sub)
sbt.index.subparser(sub)
sbt.watch.subparser(sub)
# END: dirty hacks to simultaneously support new and previous interface
Comment on lines 105 to 110

This comment has been minimized.

Copy link
@standage

standage Dec 6, 2019

Author Member

Moar dirty hacks: support both sourmash categorize and sourmash sbt categorize, both sourmash sbt_combine and sourmash sbt combine, and so on.

This comment has been minimized.

Copy link
@luizirber

luizirber Dec 6, 2019

Member

categorize, watch and index are potentially index operations, should work with lca too (so I suggest keeping it as a upper level, instead of inside sbt).

This comment has been minimized.

Copy link
@standage

standage Dec 6, 2019

Author Member

Gotcha. Well, that only leaves the sbt_combine as a candidate for grouping sbt-specific subcommands, so I'll just undo that grouping and revert those 4 subcommands to top-level commands.

subparser = subparsers.add_parser('compare')
subparser.add_argument('csv1', help='taxonomy spreadsheet output by classify')
subparser.add_argument('csv2', help='custom taxonomy spreadsheet')
subparser.add_argument(
'-q', '--quiet', action='store_true',
help='suppress non-error output'
)
subparser.add_argument(
'-d', '--debug', action='store_true',
help='output debugging output'
)
subparser.add_argument(
'-C', '--start-column', metavar='C', default=2, type=int,
help='column at which taxonomic assignments start; default=2'
)
subparser.add_argument(
'--tabs', action='store_true',
help='input spreadsheet is tab-delimited; default is commas'
)
subparser.add_argument(
'--no-headers', action='store_true',
help='no headers present in taxonomy spreadsheet'
)
subparser.add_argument('-f', '--force', action='store_true')

# Dirty hack to simultaneously support new and previous interface
# If desired, this function can be removed with a major version bump.
subparser = subparsers.add_parser('compare_csv')
subparser.add_argument('csv1', help='taxonomy spreadsheet output by classify')
subparser.add_argument('csv2', help='custom taxonomy spreadsheet')
subparser.add_argument(
'-q', '--quiet', action='store_true',
help='suppress non-error output'
)
subparser.add_argument(
'-d', '--debug', action='store_true',
help='output debugging output'
)
subparser.add_argument(
'-C', '--start-column', metavar='C', default=2, type=int,
help='column at which taxonomic assignments start; default=2'
)
subparser.add_argument(
'--tabs', action='store_true',
help='input spreadsheet is tab-delimited; default is commas'
)
subparser.add_argument(
'--no-headers', action='store_true',
help='no headers present in taxonomy spreadsheet'
)
subparser.add_argument('-f', '--force', action='store_true')
Comment on lines 2 to 52

This comment has been minimized.

Copy link
@standage

standage Dec 6, 2019

Author Member

Another example of the dirty hack scheme: duplicated subparser definitions, presumably to be removed with a major version bump.

@standage

This comment has been minimized.

Copy link
Member Author

standage commented Dec 6, 2019

Short version: intermediate review plz?

Long version: Ok, I think I've reached an inflection point. Everything I've done up to this point should be stable for the long term. Minor updates to the CLI on the master branch would have to be recapitulated in this branch, but I expect those types of changes to be infrequent and minimal.

But hooking up the new CLI to the command/subcommand implementations is going to require moving around A LOT of code, and doing so in a way that git will have difficulty tracking. I don't think we should start this process until we are confident we can resolve it quickly. Any other PRs that are merged in parallel in the mean time will have to be recapitulated here, and I'm not enthusiastic about coordinating that.

So before I proceed here, could you folks provide some feedback on what I've done so far? Any questions or concerns?

If y'all support where I'm going with this, I'll be happy to take the lead on the next steps. It'll be tedious, but I don't think it'll take an inordinate amount of time. The last thing I want to do is spend several hours updating the command/subcommand implementations, only to then spend the next few months trying to keep the PR in sync with parallel updates.

@standage

This comment has been minimized.

Copy link
Member Author

standage commented Dec 6, 2019

By the way, I've copy/pasted some terminal output above, but the best way to see how this is all working might just be to checkout the cli branch and start playing around with the sourmash command.

@luizirber

This comment has been minimized.

Copy link
Member

luizirber commented Dec 6, 2019

This is really great, @standage!

I was partially doing this with subparsers in #245, but I totally support moving all the argparsing into sourmash.cli

I was keeping the subparsers-equivalent function near the actual command to help doing proper updates to the function when changing the CLI, and then attaching it to the function (yes, yes, it's a hack) to be able to call it easily in the __main__ function. This is not needed in this PR (since it will still be passing the args object around, and not exploding it like I did here), but will make #245 easier to implement.

@luizirber

This comment has been minimized.

Copy link
Member

luizirber commented Dec 6, 2019

But hooking up the new CLI to the command/subcommand implementations is going to require moving around A LOT of code, and doing so in a way that git will have difficulty tracking. I don't think we should start this process until we are confident we can resolve it quickly. Any other PRs that are merged in parallel in the mean time will have to be recapitulated here, and I'm not enthusiastic about coordinating that.

Does it need to move code? I think we should keep the command code in sourmash.commands for now, and just call it. Like this (for compare):

diff --git a/sourmash/cli/compare.py b/sourmash/cli/compare.py
index e55b925..eaf93b8 100644
--- a/sourmash/cli/compare.py
+++ b/sourmash/cli/compare.py
@@ -36,4 +36,4 @@ def subparser(subparsers):
  
 def main(args):
-    print(args)
+    return sourmash.commands.compare(args)

UPDATE: OK, I tried this with search and it failed, and then went to check compare and... it's more complicated than just replacing. But I think removing the argument parsing from sourmash/commands.py should work with what I suggested?

If y'all support where I'm going with this, I'll be happy to take the lead on the next steps. It'll be tedious, but I don't think it'll take an inordinate amount of time. The last thing I want to do is spend several hours updating the command/subcommand implementations, only to then spend the next few months trying to keep the PR in sync with parallel updates.

Truer wods were never spoken. Keeping PR in sync with parallel updates is horrible =]

@standage

This comment has been minimized.

Copy link
Member Author

standage commented Dec 6, 2019

Does it need to move code? I think we should keep the command code in sourmash.commands for now, and just call it.

That's true. At that point, I was mixing concerns: having a functional CLI, and then being able to invoke that CLI conveniently from Python/the test suite (a la #245).

@standage

This comment has been minimized.

Copy link
Member Author

standage commented Dec 6, 2019

The suggestion of moving code referred to the design proposed at the end of this comment.

@ctb ctb changed the title Implement sane argument parsing Implement improved & consistent argument parsing Dec 7, 2019
@ctb

This comment has been minimized.

Copy link
Member

ctb commented Dec 7, 2019

(note, adjusted title)

@luizirber

This comment has been minimized.

Copy link
Member

luizirber commented Dec 23, 2019

@standage sorry for all the changes landing on master =(

But... there is a window for including this in 3.0.0-rc2, and release it together with 3.0.0. Would you be interested in working on this after the holidays? (and, if not, can I take over the branch?)

@standage

This comment has been minimized.

Copy link
Member Author

standage commented Dec 23, 2019

sorry for all the changes landing on master

No worries. As I said earlier, what I've done so far is pretty isolated from the rest of the codebase.

Would you be interested in working on this after the holidays?

Yep, I had intended to finish this already. But...you know how it goes.

if not, can I take over the branch?

If I'm ever moving too slowly, feel free to push ahead without me. 😀

@luizirber luizirber mentioned this pull request Dec 24, 2019
5 of 5 tasks complete
@luizirber

This comment has been minimized.

Copy link
Member

luizirber commented Dec 24, 2019

if not, can I take over the branch?

If I'm ever moving too slowly, feel free to push ahead without me. grinning

Taking over sounded too aggressive, so I opened #811 (merging into this one) for implementations of all commands (pretty much just calling what already exists). All tests are now working (I added back missing commands, like multigather).

Some notes:

  • duplicated commands (the function that runs after args are parsed): I'm very inclined to keep the ones in sourmash.cli, but in the unlikely case there is someone out there calling the sourmash.commands variations in their code it is easier to transition than rewriting for the new ones. The ones in sourmash.cli are very close to what I did for compute in #245 and I would like to see everything going in this directions... but maybe in sourmash 4.0?
    • describe: in sourmash.cli.sig and sourmash.sig
    • info: in sourmash.cli.info and sourmash.commands
  • can't set default for ksize in sourmash.cli.utils.add_ksize_arg. When I first saw https://github.com/dib-lab/sourmash/pull/785/files#diff-e1ba488951dcb791667fc234a1958321R83-R87 I was confused: why the default in the function call is not used in the parser? Turns out there are many places in the code that check if args.ksize is None, and then depend on that for behavior... I had to add an override in sourmash.lca.command_index.index, because it was easier than breaking/fixing all the other places where it failed.
  • I moved sourmash.cli.signature to sourmash.cli.sig, to match current CLI.
  • sbt index, sbt categorize and sbt watch were all moved back (they are index commands, not just sbt)
  • Current CLI raises SystemError(1), this PR was doing SystemError(0). I changed sourmash/cli/__init__.py a bit to account for that, and also to print the help message if invoking a command without subcommands (like sourmash sbt). Before it was throwing an exception.
  • There are two changes in tests, and an extra one. test_run_sourmash_badcmd in special is... a breaking change, even if small (the printed message changed).

Future

  • sourmash.commands and sourmash.cli have overlapping functionality, should they be merged?
  • There are many arguments that can be bundled together, and have functions like add_moltype_args and add_ksize_args for them. quiet/debug are good candidates, for example.
@luizirber luizirber marked this pull request as ready for review Dec 27, 2019
@luizirber

This comment has been minimized.

Copy link
Member

luizirber commented Dec 27, 2019

Any ideas on how to get nicer usage/help messages? Current ones for sourmash lca and sourmash sig are manually defined, and harder to keep updated... But they are nicer =]
https://github.com/dib-lab/sourmash/blob/ba5b5a1b630340f3271adf1a6f94a2a1e48e09ee/sourmash/lca/__main__.py#L12L27
https://github.com/dib-lab/sourmash/blob/ba5b5a1b630340f3271adf1a6f94a2a1e48e09ee/sourmash/sig/__main__.py#L18L39

@standage

This comment has been minimized.

Copy link
Member Author

standage commented Jan 2, 2020

Any ideas on how to get nicer usage/help messages? Current ones for sourmash lca and sourmash sig are manually defined, and harder to keep updated... But they are nicer =]

Yeah, it's a trade off: flexibility or convenience. It is possible to have some middle ground using RawDescriptionHelpFormatter, but to the extent that you manually duplicate argparse's functionality you incur the burden of maintenance.

The output of sourmash -h is currently as follows. I do think we can do better than this.

usage: sourmash [-h] [-v] [-q] cmd ...

Compute, compare, manipulate, and analyze MinHash sketches of DNA sequences.

Commands:
  Invoke "sourmash <cmd> --help" for more details on executing each command.

  cmd            categorize -- compare -- compute -- dump -- gather --
                 import_csv -- index -- info -- lca -- migrate -- multigather
                 -- plot -- sbt -- sbt_combine -- search -- sig -- storage --
                 watch

Options:
  -h, --help     show this help message and exit
  -v, --version  show program's version number and exit
  -q, --quiet    don't print citation information

Maybe with something like this, although it does run a bit long. We could easily build it automatically by recursively traversing the sourmash/cli/ directory, as long as we enforce that the directory and file names match the interface definition.

usage: sourmash [-h] [-v] [-q] cmd ...

Compute, compare, manipulate, and analyze MinHash sketches of DNA sequences.
Invoke "sourmash <cmd> --help" for more details on executing each command.

    sourmash categorize --help
    sourmash compare --help
    sourmash compute --help
    sourmash dump --help
    sourmash gather --help
    sourmash import_csv --help
    sourmash index --help
    sourmash info --help
    sourmash lca classify --help
    sourmash lca compare --help
    sourmash lca gather --help
    sourmash lca index --help
    sourmash lca rankinfo --help
    sourmash lca summarize --help
    sourmash migrate --help
    sourmash multigather --help
    sourmash plot --help
    sourmash sbt_combine --help
    sourmash search --help
    sourmash sig describe --help
    sourmash sig downsample --help
    sourmash sig export --help
    sourmash sig extract --help
    sourmash sig filter --help
    sourmash sig flatten --help
    sourmash sig ingest --help
    sourmash sig intersect --help
    sourmash sig merge --help
    sourmash sig overlap --help
    sourmash sig rename --help
    sourmash sig subtract --help
    sourmash storage convert -h --help
    sourmash watch --help

Options:
  -h, --help     show this help message and exit
  -v, --version  show program's version number and exit
  -q, --quiet    don't print citation information

We could potentially even group the commands into sections, reflecting the command/subcommand structure, with "Basic commands" being the default.

usage: sourmash [-h] [-v] [-q] cmd ...

Compute, compare, manipulate, and analyze MinHash sketches of DNA sequences.
Invoke "sourmash <cmd> --help" for more details on executing each command.

    Basic operations
        sourmash categorize --help
        sourmash compare --help
        sourmash compute --help
        sourmash dump --help
        sourmash gather --help
        sourmash import_csv --help
        sourmash index --help
        sourmash info --help
        sourmash migrate --help
        sourmash multigather --help
        sourmash plot --help
        sourmash sbt_combine --help
        sourmash search --help
        sourmash watch --help

    Lowest common ancestor (LCA) based operations
        sourmash lca classify --help
        sourmash lca compare --help
        sourmash lca gather --help
        sourmash lca index --help
        sourmash lca rankinfo --help
        sourmash lca summarize --help
    
    Operations on signatures (MinHash sketches)
        sourmash sig describe --help
        sourmash sig downsample --help
        sourmash sig export --help
        sourmash sig extract --help
        sourmash sig filter --help
        sourmash sig flatten --help
        sourmash sig ingest --help
        sourmash sig intersect --help
        sourmash sig merge --help
        sourmash sig overlap --help
        sourmash sig rename --help
        sourmash sig subtract --help
    
    Operations on storage
        sourmash storage convert -h --help

Options:
  -h, --help     show this help message and exit
  -v, --version  show program's version number and exit
  -q, --quiet    don't print citation information

By the way, are additional sourmash storage subcommands planned? At the moment it appears that convert is the only one implemented.

@luizirber

This comment has been minimized.

Copy link
Member

luizirber commented Jan 2, 2020

Yeah, it's a trade off: flexibility or convenience. It is possible to have some middle ground using RawDescriptionHelpFormatter, but to the extent that you manually duplicate argparse's functionality you incur the burden of maintenance.

I started doing one that digged into the argparse internals and... it got ugly fast. =/

We could potentially even group the commands into sections, reflecting the command/subcommand structure, with "Basic commands" being the default.

Ooooh, this one is NICE!

By the way, are additional sourmash storage subcommands planned? At the moment it appears that convert is the only one implemented.

Yes, I'll be more mindful when adding commands in the future... for now it's better to keep what exist working, and then remove on 4.0 if no other commands show up.

@standage

This comment has been minimized.

Copy link
Member Author

standage commented Jan 2, 2020

Ok, sourmash --help now produces this.

usage: sourmash [-h] [-v] [-q]

Compute, compare, manipulate, and analyze MinHash sketches of DNA sequences.

    Basic operations
        sourmash categorize --help
        sourmash compare --help
        sourmash compute --help
        sourmash dump --help
        sourmash gather --help
        sourmash import_csv --help
        sourmash index --help
        sourmash info --help
        sourmash migrate --help
        sourmash multigather --help
        sourmash plot --help
        sourmash sbt_combine --help
        sourmash search --help
        sourmash watch --help

    Lowest common ancestor (LCA) based operations
        sourmash lca classify --help
        sourmash lca compare --help
        sourmash lca gather --help
        sourmash lca index --help
        sourmash lca rankinfo --help
        sourmash lca summarize --help

    Operations on signatures (MinHash sketches)
        sourmash sig describe --help
        sourmash sig downsample --help
        sourmash sig export --help
        sourmash sig extract --help
        sourmash sig filter --help
        sourmash sig flatten --help
        sourmash sig ingest --help
        sourmash sig intersect --help
        sourmash sig merge --help
        sourmash sig overlap --help
        sourmash sig rename --help
        sourmash sig subtract --help

    Operations on storage
        sourmash storage convert --help

Instructions:
  Invoke "sourmash <cmd> --help" or "sourmash <cmd> <subcmd> --help"
  for more details on executing each command.

Options:
  -h, --help     show this help message and exit
  -v, --version  show program's version number and exit
  -q, --quiet    don't print citation information

Command groups have a similar summary. sourmash lca --help produces this.

usage: sourmash lca [-h]

Subcommands:
  Invoke "sourmash lca <subcmd> --help" for more details on executing each subcommand.
  
      Operations
          sourmash lca classify --help
          sourmash lca compare --help
          sourmash lca gather --help
          sourmash lca index --help
          sourmash lca rankinfo --help
          sourmash lca summarize --help

Options:
  -h, --help  show this help message and exit

And finally, each operation has its own usage. sourmash lca index --help produces this.

usage: sourmash lca index [-h] [--scaled S] [-k K] [-q] [-d] [-C C] [--tabs]
                          [--no-headers] [--split-identifiers] [-f]
                          [--traverse-directory] [--report REPORT]
                          [--require-taxonomy]
                          csv lca_db_out signatures [signatures ...]

positional arguments:
  csv                   taxonomy spreadsheet
  lca_db_out            output database name
  signatures            one or more sourmash signatures

optional arguments:
  -h, --help            show this help message and exit
  --scaled S
  -k K, --ksize K       k-mer size; default=31
  -q, --quiet           suppress non-error output
  -d, --debug           output debugging output
  -C C, --start-column C
                        column at which taxonomic assignments start; default=2
  --tabs                input spreadsheet is tab-delimited; default is commas
  --no-headers          no headers present in taxonomy spreadsheet
  --split-identifiers   split names in signatures on whitspace and period
  -f, --force
  --traverse-directory  load all signatures underneath directories
  --report REPORT       output a report on anomalies, if any
  --require-taxonomy    ignore signatures with no taxonomy entry
@ctb

This comment has been minimized.

Copy link
Member

ctb commented Jan 2, 2020

@ctb

This comment has been minimized.

Copy link
Member

ctb commented Jan 2, 2020

self.citation = citation

@classmethod
def print_citation(cls):

This comment has been minimized.

Copy link
@luizirber

luizirber Jan 8, 2020

Member

I made print_citation a class method, and storing the _citation_printed at the class level let's us assert that it is only printed once along all commands (because all commands are subclasses of SourmashParser).

(the classmethod trick is straight out of snakemake, btw)

@luizirber luizirber merged commit 585108f into master Jan 8, 2020
19 of 20 checks passed
19 of 20 checks passed
Check
Details
build
Details
test (beta)
Details
test (stable)
Details
test (windows)
Details
test (macos)
Details
test_all_features
Details
coverage
Details
Lints
Details
Check if wasm-pack builds a valid package for the sourmash crate
Details
Run tests under wasm32-wasi Run tests under wasm32-wasi
Details
Publish (dry-run)
Details
minimum_rust_version
Details
Travis CI - Pull Request Build Errored
Details
LGTM analysis: C/C++ No code changes detected
Details
LGTM analysis: JavaScript No code changes detected
Details
LGTM analysis: Python 8 new alerts
Details
codecov/patch 97.46% of diff hit (target 79.21%)
Details
codecov/project 79.36% (+0.14%) compared to 6a2a14e
Details
netlify/sourmash-docs/deploy-preview Deploy preview canceled.
Details
@luizirber luizirber deleted the cli branch Jan 8, 2020
@luizirber

This comment has been minimized.

Copy link
Member

luizirber commented Jan 8, 2020

🎉 Thanks @standage 💯

@ctb

This comment has been minimized.

Copy link
Member

ctb commented Jan 8, 2020

@standage

This comment has been minimized.

Copy link
Member Author

standage commented Jan 8, 2020

Awesome. Thanks for your feedback!

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