lazear / sage

Proteomics search & quantification so fast that it feels like magic
https://sage-docs.vercel.app
MIT License
210 stars 39 forks source link

Redundancy between --fasta CLI argument and required fasta path in config file? #83

Closed j-berg closed 1 year ago

j-berg commented 1 year ago

Is there a reason the FASTA path is required in the config file while it can be provided as a command line argument? I feel like it would make more sense - since it is a CLI argument - to let the user provide the path to the fasta with the CLI and/or the config file. Happy to open a pull request if you agree.

lazear commented 1 year ago

The FASTA path is not actually required in the config file, this is a just a holdover/bug in the docs that was never updated when the CLI argument was added 😉 . If the user provides the path via CLI, then it overwrites anything in the config file.

If you want to submit a PR to fix, I will merge it. Otherwise, I can update the README.

j-berg commented 1 year ago

I see. That's my bad - I forgot JSON files don't like trailing commas, which I had when I removed the FASTA path. On re-reading the README with this as the cause of the error in mind, the docs are actually clear and was just a syntax error on my part. Thanks!

Screenshot 2023-08-23 at 3 26 27 PM Screenshot 2023-08-23 at 3 25 41 PM