Allow fingerprinting of SAM files that only have a partial dictionary match to the haplotype map #1955
Conversation
…ting sam files that have a mismatching dictionary from the one stated in the haplotype map (it works for vcfs). This is because when fingerprinting sam files, the SamLocusIterator is given an IntervalList to query, and this IL has a mismatching dictionary which isn't supported. This PR resolves this by creating a new IL just before querying the Iterator using the proper dictionary.
| @Test(dataProvider = "bamFilesRGs") | ||
| public void testCrossCheckRGs_with_short_dictionary(final File file1, final File file2, final boolean expectAllMatch, final int expectedRetVal, final int expectedNMetrics) throws IOException { | ||
|
|
||
| File metrics = File.createTempFile("Fingerprinting", "NA1291.RG.crosscheck_metrics"); |
|
|
||
| if (file1.getName().endsWith(SamReader.Type.CRAM_TYPE.fileExtension())) { | ||
| args.add("R=" + referenceForCrams); | ||
| args.add("HAPLOTYPE_MAP=" + HAPLOTYPE_MAP_FOR_CRAMS); |
There was a problem hiding this comment.
could you add a HAPLOTYPE_MAP_SHORT_FOR_CRAMS as well just so we're testing that the functionality works for both bams and crams?
|
Hi @kachulis I'm not sure what the failure it about, but I'm pretty sure it's not due to my changes.... |
| private Object[][] haplotypeMaps(){ | ||
| return new Object[][]{{HAPLOTYPE_MAP}, {HAPLOTYPE_MAP_SHORT}}; | ||
| } | ||
| @DataProvider(name = "bamFilesRGs") |
There was a problem hiding this comment.
I think this has gotten a little twisted around. You're testing the both full and short dictionary here, but only the full dictionary crams, and then you also have testCrossCheckRGs_with_short_dictionary, where you basically do the same thing, expect hardcoded instead of via the data provider. I think that's also where the test failure is coming from, bamFilesRGs now includes the haplotypeMaps, but testCrossCheckRGs_with_short_dictionary doesn't include the haplotypeMaps as a parameter input.
There was a problem hiding this comment.
😊 huh. I saw other errors before. yes, something got strange.
|
OK. back to you again @kachulis . thanks for your patience. |
|
I've seems to have lost my merge privileges....be my guest! |
Despite efforts to the contrary, FingerprintChecker cannot fingerprinting sam files that have a mismatching dictionary from the one stated in the haplotype map (it works for vcfs). This is because when fingerprinting sam files, the SamLocusIterator is given an IntervalList to query, and this IL has a mismatching dictionary which isn't supported.
This PR resolves this by creating a new IL just before querying the Iterator using the proper dictionary.
Description
Give your PR a concise yet descriptive title
Please explain the changes you made here.
Explain the motivation for making this change. What existing problem does the pull request solve?
Mention any issues fixed, addressed or otherwise related to this pull request, including issue numbers or hard links for issues in other repos.
You can delete these instructions once you have written your PR description.
Checklist (never delete this)
Never delete this, it is our record that procedure was followed. If you find that for whatever reason one of the checklist points doesn't apply to your PR, you can leave it unchecked but please add an explanation below.
Content
Review
For more detailed guidelines, see https://github.com/broadinstitute/picard/wiki/Guidelines-for-pull-requests