-
Notifications
You must be signed in to change notification settings - Fork 597
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
Exposed maximum copy ratio and point size for CNV plotting tools. #6482
Conversation
fd64811
to
387e0a7
Compare
@bhanugandham Comms may be interested in increasing the |
@fleharty mind reviewing? This is pretty boilerplate, but it's possible that I've erred in my copy-paste-editing somewhere... |
387e0a7
to
8cd4b4a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, my only question is whether or not you should use "--" or "-" in the StandardArgumentDefinitions.
@Test(expectedExceptions = IllegalArgumentException.class) | ||
public void testMinimumContigLength() { | ||
final File outputDir = createTempDir("testDir"); | ||
final String[] arguments = { | ||
"--" + CopyNumberStandardArgument.STANDARDIZED_COPY_RATIOS_FILE_LONG_NAME, STANDARDIZED_COPY_RATIOS_FILE.getAbsolutePath(), | ||
"--" + CopyNumberStandardArgument.DENOISED_COPY_RATIOS_FILE_LONG_NAME, DENOISED_COPY_RATIOS_FILE.getAbsolutePath(), | ||
"-" + StandardArgumentDefinitions.SEQUENCE_DICTIONARY_NAME, SEQUENCE_DICTIONARY_WITH_NO_CONTIGS_ABOVE_MINIMUM_LENGTH_FILE.getAbsolutePath(), | ||
"--" + StandardArgumentDefinitions.SEQUENCE_DICTIONARY_NAME, SEQUENCE_DICTIONARY_WITH_NO_CONTIGS_ABOVE_MINIMUM_LENGTH_FILE.getAbsolutePath(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this StandardArgumentDefinitions us "--" but the next one for OUTPUT_SHORT_NAME uses only "-".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Short names use -
, long names use --
. It just happens that the short and long names for sequence dictionary are the same for the CNV plotting tools, but this line of code would've broken if that was ever changed.
@@ -81,7 +99,7 @@ public void testNonExistentStandardizedFile() { | |||
final String[] arguments = { | |||
"--" + CopyNumberStandardArgument.STANDARDIZED_COPY_RATIOS_FILE_LONG_NAME, "Non-existent-file", | |||
"--" + CopyNumberStandardArgument.DENOISED_COPY_RATIOS_FILE_LONG_NAME, DENOISED_COPY_RATIOS_FILE.getAbsolutePath(), | |||
"-" + StandardArgumentDefinitions.SEQUENCE_DICTIONARY_NAME, SEQUENCE_DICTIONARY_FILE.getAbsolutePath(), | |||
"--" + StandardArgumentDefinitions.SEQUENCE_DICTIONARY_NAME, SEQUENCE_DICTIONARY_FILE.getAbsolutePath(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do StandardArgumentDefinitions use "-" or "--"?
Closes #6391.
Closes #5748.
Note that the *.denoisedLimit4.png output has been removed from the PlotDenoisedCopyRatios tool and the corresponding WDL task. The default behavior has changed slightly, in that the remaining *.denoised.png output is now delimited to maximum copy ratio = 4.0 (instead of covering the entire range in the data). @fleharty @droazen we may want to mention this in the release notes.