-
Notifications
You must be signed in to change notification settings - Fork 596
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
Remove trailing tab in VariantsToTable output header #7559
Conversation
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.
The fix to the tool looks good @kachulis, but one question for you about the test code
@@ -63,6 +64,7 @@ public IntegrationTestSpec(String args, List<String> expectedFileNames) { | |||
this.compareBamFilesSorted = false; | |||
this.validationStringency = ValidationStringency.DEFAULT_STRINGENCY; | |||
this.tempExtension = DEFAULT_TEMP_EXTENSION; | |||
this.trimWhiteSpace = true; |
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.
Was IntegrationTestSpec
previously defaulting to trimming whitespace before doing the comparison vs the expected output(s), or is this PR introducing a change in default behavior?
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.
@droazen previously (and still after this pr) IntegrationTestSpec defaulted to trimming whitespace. Which is how this bug was never caught by tests before.
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.
Ah, I see...in that case I think I'll open a separate ticket to see if we can change the default for IntegrationTestSpec
to not ignore leading/trailing whitespace. Go ahead and merge once tests pass 👍
If VariantsToTable is run without any genotype field outputs (or on a vcf with no sample data), the header line contains an extra trailing tab, which is not included in the table rows. This confuses many tsv parsers, which see the header as having an extra (blank) field, and so complain that the rows of the table do not contain enough fields.
This pr fixes this so that there is no longer a trailing tab in this case.