Skip to content

Modern arguments #1216

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

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

Modern arguments #1216

wants to merge 17 commits into from

Conversation

rambaut
Copy link
Contributor

@rambaut rambaut commented Jan 25, 2025

This pull request updates all the command line options to use the 'modern' convention of using -- for a long form command line option and - for a shortened equivalent. Note that the original options using a single - with the long form also work for backwards compatibility.

This could wait until after the BEAST X 10.5.1 release.

@rambaut rambaut requested a review from Copilot April 12, 2025 10:15
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 47 out of 47 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

src/dr/app/tools/Branch2dRateToGrid.java:1479

  • The description for the 'TRAIT_NOISE' option includes an extra closing parenthesis. Consider removing the extra ')' to maintain consistency.
new Arguments.StringOption(TRAIT_NOISE, null, falseTrue, false, "add true noise to 2D traits [default = true])")

src/dr/app/beast/BeastMain.java:397

  • The shorthand 'bi' is reused for both 'beagle_info' and 'beagle_instances', which could lead to ambiguity. Consider using a unique shorthand flag for 'beagle_instances'.
new Arguments.IntegerOption("beagle_instances", "bi", "BEAGLE: divide site patterns amongst instances"),

@rambaut rambaut requested a review from Copilot April 12, 2025 10:39
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 47 out of 47 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

src/dr/app/phylogeography/tools/DiscreteTreeToKML.java:277

  • The help message for the TEMP option appears to be missing a closing bracket (']') in its default value description.
new Arguments.StringOption(TEMP, null, falseTrue, false, "display branches only temporary [default=false")

src/dr/app/beast/BeastMain.java:370

  • The short flag 's' for the 'strict' option conflicts with another option; consider assigning a unique short flag to avoid ambiguity.
new Arguments.Option("strict", "s", "Fail on non-conforming BEAST XML file")

src/dr/app/beast/BeastMain.java:374

  • The short flag 's' for the 'seed' option conflicts with the 'strict' option; a unique short flag should be used here to prevent conflicts.
new Arguments.LongOption("seed", "s", "Specify a random number generator seed")

new Arguments.StringOption(CLADESTEM, null, falseTrue, false, "include clade stem [default=false]"),
new Arguments.StringOption(EXCLUDECLADESTEM, null, falseTrue, true, "include clade stem in the exclusion [default=true]"),
new Arguments.StringOption(BACKBONETAXA,null, "Backbone taxa file", "specifies a file with taxa that define the backbone"),
new Arguments.StringOption(ZEROBRANCHES,null, falseTrue, true, "include branches with 0 N and S subtitutions [default=included]"),
Copy link
Preview

Copilot AI Apr 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The help text for ZEROBRANCHES contains a typo ('subtitutions'); it should be 'substitutions'.

Suggested change
new Arguments.StringOption(ZEROBRANCHES,null, falseTrue, true, "include branches with 0 N and S subtitutions [default=included]"),
new Arguments.StringOption(ZEROBRANCHES,null, falseTrue, true, "include branches with 0 N and S substitutions [default=included]"),

Copilot uses AI. Check for mistakes.

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.

1 participant