sourmash-bio / sourmash

Quickly search, compare, and analyze genomic and metagenomic data sets.
http://sourmash.readthedocs.io/en/latest/
Other
467 stars 79 forks source link

Abundance argument mismatch on `sourmash sig intersect` #2291

Open ccbaumler opened 1 year ago

ccbaumler commented 1 year ago

When attempting to intersect signature files with preserved abundance, I followed this documentation and found the following error/results.

$ sourmash signature intersect --abundance-from SRR5962884.sig merged.sig -o intersect.sig
sourmash: error: unrecognized arguments: --abundance-from

$ sourmash signature intersect -A SRR5962884.sig merged.sig -o intersect.sig

== This is sourmash version 4.5.0. ==
== Please cite Brown and Irber (2016), doi:10.21105/joss.00027. ==

loaded 1 signatures total, from 1 files
loading signature from SRR5962884.sig, keeping abundances
loaded and intersected 1 signatures

At the intersect of main and intersect.py, the argument is named abundances-from. There is a further discrepancy between --ignore-abundance in the documentation as well.

I purpose all --abundance related commands be a uniform singular tense (E.g. --track-abundance, --ignore-abundance, --min-abundance, --max-abundance, --abundance-from, --with-abundance).

ctb commented 1 year ago

singular sounds good to me! however per semantic versioning we can't remove --abundances-from without bumping the major version, we can only make --abundance-from work (which should be as simple as adding it to the add_argument call). so I'd suggest that.

note that a feature of argparse is that it does partial string matching, so --abund would work as long as there's a single argument that starts with abund. This means --track-abundance would match --track-abundances, etc - so really only --abundance-from is suffering bigly from this ambiguity.

you might also remove the [WIP] from this as this is an issue not a PR :)