More informative error messages when dictionaries don't match #1870
Conversation
This file contains 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
kachulis
commented
May 10, 2023
| SequenceUtil.assertSequenceDictionariesEqual(SAMSequenceDictionaryExtractor.extractDictionary(inputPath), SAMSequenceDictionaryExtractor.extractDictionary(genotypesPath), true); | ||
| SequenceUtil.assertSequenceDictionariesEqual(SAMSequenceDictionaryExtractor.extractDictionary(inputPath), checker.getHeader().getSequenceDictionary(), true); | ||
|
|
||
| try { |
There was a problem hiding this comment.
this should be able to switch to SequenceDictionaryUtils.assertSequenceDictionariesEqual as well, right?
There was a problem hiding this comment.
I didn't change these because they use the extra checkPrefixOnly argument, which the others don't. So rather than create another overload in Picard, I left these as direct calls to htsjdk. If you think its worth it though I could add a second method for those cases though.
There was a problem hiding this comment.
ahh, I see. No I think that's fine in that case.
kachulis
commented
May 10, 2023
| throw new IllegalArgumentException("Dictionary on fingerprinted file smaller than that on Haplotype Database!"); | ||
| throw new SequenceUtil.SequenceListsDifferException("Dictionary on fingerprinted file smaller than that on Haplotype Database!"); | ||
| } | ||
| try { |
|
@cmnbroad looks like I can't approve this since I originally submitted it, but this looks good to me. |
cmnbroad
approved these changes
May 11, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Addresses common complaint about opaque error messages when sequence dictionaries don't match, by specifying which files the mismatched sequence dictionaries are coming from.
A recent example of this issue is #1869