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

Improvements in CLI & additional subcommands #90

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

OndrejSladky
Copy link
Owner

@OndrejSladky OndrejSladky commented Feb 12, 2025

  • Improved CLI structure
  • Improved output headers
  • Added subcommands for conversion to/from SPSS and splitting/joining ms.
  • Possibility to construct maxone mask during masked superstring construction.

@OndrejSladky OndrejSladky linked an issue Feb 12, 2025 that may be closed by this pull request
@OndrejSladky OndrejSladky linked an issue Feb 17, 2025 that may be closed by this pull request
@OndrejSladky OndrejSladky changed the title Improvements in CLI & additional subcommands [wip] Improvements in CLI & additional subcommands Feb 17, 2025
@PavelVesely
Copy link
Collaborator

A small glitch in naming the commands: the ms command outputs a .msfa file with mask-cased superstring , whereas in commands ms2msfa and msfa2ms the MS abbrev refers to separate superstring and mask. Not sure how to make it better...

@OndrejSladky
Copy link
Owner Author

Yeah, I do agree with this, I could rename it to sth like compute which'd be more descriptive than msfa.

@OndrejSladky
Copy link
Owner Author

Also I'm not sure, if -M should not output the masked-cased superstring maximizing ones instead of the mask, as it feels kinda anoying to run msfa2ms and then ms2msfa to obtain it.

@PavelVesely
Copy link
Collaborator

Also I'm not sure, if -M should not output the masked-cased superstring maximizing ones instead of the mask, as it feels kinda anoying to run msfa2ms and then ms2msfa to obtain it.

Yes, I think -M should output a .msfa file as it can then be used in FMSI-memb directly (right, it'd be annoying to run to more commands to obtain it).

@karel-brinda
Copy link
Collaborator

karel-brinda commented Feb 21, 2025

Comments:

  • agree with compute
  • optimize is misleading - probably maskopt?
  • ms2msfa - Split a masked superstring into a superstring and a mask. - should be the other way around
  • ./kmercamel msfa2ms doesn't contain any message
  • is there any better way how to call M+S separately? Maybe mssep? It would be nice to change ms->mssep, msfa-> ms so that the basic format is the most easy to write.

@karel-brinda
Copy link
Collaborator

Unless it's critical for the current submission, we can finalize it next week.

@PavelVesely
Copy link
Collaborator

I will try to use the new CLI and streaming in a new version of the experimental pipeline, hopefully this week. Some renaming is still fine

@karel-brinda
Copy link
Collaborator

karel-brinda commented Feb 24, 2025

@OndrejSladky Could you give us a rough timeline for this? So we know what kind of additional feedback you need from us at each stage.

By the way, I’m really looking forward to seeing this integrated! :)

@OndrejSladky
Copy link
Owner Author

@OndrejSladky Could you give us a rough timeline for this? So we know what kind of additional feedback you need from us at each stage.

By the way, I’m really looking forward to seeing this integrated! :)

I'll update the changes you've suggested, likely tomorrow, and then we can iterate other suggestions on how the commands should be named and similar, until we're all satisfied. Then I'll merge (likely including the streaming alg) and bump to v2.0.0 fixing the interface.

@OndrejSladky
Copy link
Owner Author

Comments:

* agree with `compute`

* `optimize` is misleading - probably `maskopt`?

* `ms2msfa    - Split a masked superstring into a superstring and a mask.` - should be the other way around

* `./kmercamel msfa2ms` doesn't contain any message

* is there any better way how to call M+S separately? Maybe mssep? It would be nice to change `ms`->`mssep`, `msfa`-> `ms` so that the basic format is the most easy to write.

Agree with all of them. mssep sounds good.

@OndrejSladky OndrejSladky changed the title Improvements in CLI & additional subcommands Improvements in CLI & additional subcommands [wip] Feb 25, 2025
@OndrejSladky OndrejSladky changed the title Improvements in CLI & additional subcommands [wip] Improvements in CLI & additional subcommands Feb 25, 2025
@OndrejSladky
Copy link
Owner Author

I renamed everything as discussed and also made -M parameter output the masked superstring and not only the mask.
Additionally, I tried to improve the header and comments of the fastas outputted.
If you could take a look if now it looks good for you, then I'll merge.

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 this pull request may close these issues.

Implement conversion subcommands CLI: Adding subcommand structure and unifying file suffixes
3 participants