Updates to bring code and documentation in sync#299
Open
tfenne wants to merge 5 commits into
Open
Conversation
The docs (and generated website) advertise -T as a short flag for --threads on every tool, and -I for ligate's --input, but neither was ever declared in the source, so passing them errored out. Declare them so the binaries match the documentation. The flags are purely additive and conflict-free (no tool already used -T or, for ligate, -I).
The --bins description pointed users at --use-ref-alt and the --min-val-dp description at --minDP; neither option exists. The real options are --use-alt-af and --min-val-dp respectively.
Bring the documentation tables in line with what the binaries actually accept: - ligate: drop the --no-index row (removed from code in f310862 when streaming indexing became automatic); fix the --thread typo to --threads. - phase: --burn-in -> --burnin (matches the actual option name); correct --pbwt-modulo-cm default 5 -> 0.1; drop the bogus '5' default on the --state-list flag; document the previously-undocumented --use-gl-indels, --checkpoint-file-in and --checkpoint-file-out options. - chunk: correct the window/buffer defaults to the current values set in ab64d68 (window-cm 2.5, window-mb 2.0, window-count 20000, buffer-mb 0.4, buffer-count 2000). - concordance: --samples takes a FILE argument; fix the --bins help text reference --use-ref-alt -> --use-alt-af. - split_reference: cosmetic --threads spacing.
Per CodeRabbit: the --min-val-gl and --min-val-dp help text (source and docs) pointed at --gt-validation, which is not a real option; the flag is --gt-val. Also fixes the 'no filter of if using' typo to 'no filter if using'.
Per CodeRabbit: the rows touched in this PR lacked a trailing pipe, tripping MD055. Add trailing pipes to all data rows in the two tables this PR edits so each table is internally consistent. The pre-existing BAM/CRAM table is left untouched (out of scope for this PR).
Collaborator
|
Thanks. Will try to find some time to check this and get back to you by the end of the week! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
I keep getting bitten by the fact that the doc website documents a
--no-indexoption forGLIMPSE2_ligatewhich no longer exists in the binary. Rather than just fixing that one discrepancy, I did a full audit of the command-line options for all five tools across three sources: the option declarations in the source (*_parameters.cpp), each binary's--help, and the documentation tables indocs/docs/documentation/*.md(which generate the website).The website and the repo
.mdfiles are identical, so every gap was docs vs. code. This PR brings the two back into sync, and adds a few short flags that the docs already advertised but the binaries never accepted.Changes
Code — make the binaries match the documented behaviour
-Tshort flag for--threadson all five tools, and-IforGLIMPSE2_ligate --input. These were shown in the docs but never declared, so passing them errored out. The flags are purely additive and conflict-free (no tool already used-T, and ligate had no-I).--helpstrings inGLIMPSE2_concordancethat referenced non-existent flags:--binspointed at--use-ref-alt(real option:--use-alt-af);--min-val-gland--min-val-dppointed at--gt-validation(real option:--gt-val);--min-val-dpalso referenced--minDP. Also fixed a "no filter of if using" typo.Docs — sync the tables to the current code
--no-indexrow (the option was removed when ligation switched to automatic streaming indexing); fix the--thread→--threadstypo.--burn-in→--burnin(the documented name is rejected by the binary); correct the--pbwt-modulo-cmdefault5→0.1; drop the bogus5default shown on the--state-listflag; and document three options that were missing entirely:--use-gl-indels,--checkpoint-file-in,--checkpoint-file-out.window-cm2.5,window-mb2.0,window-count20000,buffer-mb0.4,buffer-count2000).--samplestakes a FILE argument; mirror the--use-alt-afhelp-text fix.--threadsspacing in split_reference).Deliberately not changed
--no-indexwas not re-added — it was intentionally removed and streaming indexing makes it unnecessary; only the stale doc row is dropped.Verification
All five tools build cleanly and each binary's
--helpwas confirmed to match the updated docs, including the new-T/-Ishort flags and the corrected help strings. (There is no unit-test framework in the repo; build +--helpinspection is the verification path.)