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

provide sensible default signature names for sourmash sig intersect #1801

Closed
ctb opened this issue Jan 18, 2022 · 5 comments · Fixed by #3162
Closed

provide sensible default signature names for sourmash sig intersect #1801

ctb opened this issue Jan 18, 2022 · 5 comments · Fixed by #3162

Comments

@ctb
Copy link
Contributor

ctb commented Jan 18, 2022

right now, many of the sig subcommands don't explicitly name the output signatures. Maybe this should change where sensible names can be inferred?

see discussion in #1171 (comment)

@taylorreiter
Copy link
Contributor

could we add the --name parameter to the command line? I don't dislike the default name, but it would be nice to set the name manually.

@ctb
Copy link
Contributor Author

ctb commented Feb 28, 2022

two or three thoughts -

sure, it's easy enough :). but why not use sourmash sig merge ... | sourmash sig rename?

Probably part of the problem is that --name isn't part of the set of options that work by default across all of the sourmash sig commands. If we put in the effort to add it to sourmash sig merge we should consider (a) what other good default options there should be and (b) implementing them in a "standard" way for all of the sig subcommands, somewhat like we've done with -q/--quiet, -k, and the molecule ypes.

Another part of the problem is that the default naming scheme is kinda not good here; it just names it the md5sum. Maybe instead of choosing the default we could print out something helpful like naming it default; use --name to set the name.

@taylorreiter
Copy link
Contributor

I was mostly just surprised that I got an error with --name using sourmash sig intersect; seems like there is more to consider than just adding the param. I guess rename is fine for this, a little annoying to have two steps but better then not having any options :)

@ctb ctb changed the title provide sensible default names for sourmash sig intersect provide sensible default signature names for sourmash sig intersect May 4, 2022
@ctb
Copy link
Contributor Author

ctb commented May 14, 2024

Diving into this a little - some fun facts.

  • sig merge takes --name as an argument to name the merged signature.
  • sig extract, sig filter, and sig flatten take --name arguments that select the signatures to operate on. I don't know if we've ever used this. Might be good to change it up to use some of the other sig selection mechanisms (but, major version change) or eliminate it since sig grep supports a more flexible signature selection mechanism anyway;
  • I think sig inflate, sig filter, sig downsample, and sig flatten keep the signature name from the signature they're operating on; double check this and make sure it's tested!

So candidates for a --name or --rename argument (joining sig merge) are:

  • sig intersect
  • sig subtract

which is frankly much simpler than I was expecting.

@ctb
Copy link
Contributor Author

ctb commented Jun 11, 2024

sig intersect and sig subtract support --set-name as of today's release of sourmash v4.8.9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants