Allow LocatableXsvFuncotationFactory to read gzipped files #8363

bbimber · 2023-06-14T13:54:15Z

No description provided.

bbimber · 2023-06-14T15:13:00Z

Hello,

@jonn-smith and @lbergelson: I'd like to make a custom data source using tabular data as the input. These data should be matched on position, not considering allele, using LocatableXSV as the type. There is an example of a TSV using Oreganno in the docs (https://gatk.broadinstitute.org/hc/en-us/articles/360035889931-Funcotator-Information-and-Tutorial).; however, I ran into two issues:

A TSV needs to be indexed to be read. It's not clear how to generate an idx file from a non-bgzipped tsv. GATK's IndexFieldFile does not recognize basic TSVs as input files. Perhaps I'm missing something.
It would be possible to gzip the TSV and make a tabix index. The problem is that while most GATK code seamlessly handles unzipped or gzipped inputs, LocatableXsvFuncotationFactory expected unzipped. This is a minor change to the file reading code that allows gzipped TSV inputs.

Below is an example input. You can bgzip this and index using:

tabix textSource.txt.gz -s 1 -b 2 -e 3 -S 1 -f

With this PR, I think funcotator will now support gzipped LocatableXSV sources. Would it be possible to add this?

textSource.txt

jonn-smith · 2023-06-14T15:30:00Z

@bbimber This looks like a good change, but I don't think it'll solve the problem.

The XsvLocatableTableCodec works differently than other codecs. It was essentially created for Funcotator datasources, and it expects to be given a .config file rather than the XSV file itself. For example, if you wanted to index the Oreganno data source file, you'd need to first create a funcotator configuration file adjacent to it, and then use IndexFeatureFile to index the config file rather than the tsv.

This is not a good design (my fault), but it's how the tool operates as of right now.

bbimber · 2023-06-14T17:10:17Z

@jonn-smith, I did see XsvLocatableTableCodec and the .config file path, but this does not appear to work. To be clear this is something like:

gatk IndexFeatureFile -I ./hg19/testTextSource.config

In IndexFeatureFile (

gatk/src/main/java/org/broadinstitute/hellbender/tools/IndexFeatureFile.java

Line 118 in abe8148

    
           return IndexFactory.createDynamicIndex(featurePath.toPath(), codec, IndexFactory.IndexBalanceApproach.FOR_SEEK_TIME);

), it does identify the correct codec; however, it then calls:

IndexFactory.createDynamicIndex(featurePath.toPath(), ...)

where featurePath is the config file. This calls IndexFactory to open a lineReader on the config file (not the backing data source): https://github.com/samtools/htsjdk/blob/6d3fc7bc1f613ecfce1c22d368f3ae17cb86823d/src/main/java/htsjdk/tribble/index/IndexFactory.java#L598.

This then fails during XsvLocatableTableCodec.readActualHeader(), since this is trying to read the config file, not the TXT file.

bbimber · 2023-06-14T17:17:25Z

@jonn-smith also, I've tried this locally and I'm pretty sure the change I propose here allows Funcotator to use txt.gz LocatableXSV sources, where that file is just indexed separately with tabix

bbimber · 2023-06-21T22:20:05Z

@jonn-smith: any thoughts on the PR? I could try to put in a test, but this does allow Funcotator to read gzipped TSV files, which is rather useful when you're dealing with large inputs.

droazen

This change seems fine to me @bbimber, provided you add an integration test demonstrating the new gzip support in Funcotator.

bbimber · 2023-06-22T15:47:41Z

ok

bbimber · 2023-06-22T20:54:12Z

@droazen: test added (technically a unit test), but I dont think I'm able to kick off the test suite.

bbimber · 2023-06-22T22:57:41Z

I added one unrelated bugfix. FuncotatorUtils.createReferenceSnippet tries to expand the reference window. When doing this, it should never allow a start less than 1. The last commit addresses that.

Note: I did not see an easy way for createReferenceSnippet() to identify the length of the contig (such as access to the SequenceDictionary), but it would in theory be useful to also check contig size and not exceed it.

@droazen or @jonn-smith: it would be helpful if you could approve the test run

22 Jun 2023 14:54:27,152 DEBUG: 	java.lang.IllegalArgumentException: Invalid interval. Contig:MT start:0 end:20
22 Jun 2023 14:54:27,154 DEBUG: 		at org.broadinstitute.hellbender.utils.Utils.validateArg(Utils.java:804)
22 Jun 2023 14:54:27,155 DEBUG: 		at org.broadinstitute.hellbender.utils.SimpleInterval.validatePositions(SimpleInterval.java:59)
22 Jun 2023 14:54:27,156 DEBUG: 		at org.broadinstitute.hellbender.utils.SimpleInterval.<init>(SimpleInterval.java:35)
22 Jun 2023 14:54:27,158 DEBUG: 		at org.broadinstitute.hellbender.tools.funcotator.FuncotatorUtils.createReferenceSnippet(FuncotatorUtils.java:1461)
22 Jun 2023 14:54:27,159 DEBUG: 		at org.broadinstitute.hellbender.tools.funcotator.dataSources.gencode.GencodeFuncotationFactory.createIgrFuncotation(GencodeFuncotationFactory.java:2481)
22 Jun 2023 14:54:27,160 DEBUG: 		at org.broadinstitute.hellbender.tools.funcotator.dataSources.gencode.GencodeFuncotationFactory.createIgrFuncotations(GencodeFuncotationFactory.java:2407)
22 Jun 2023 14:54:27,162 DEBUG: 		at org.broadinstitute.hellbender.tools.funcotator.dataSources.gencode.GencodeFuncotationFactory.createDefaultFuncotationsOnVariant(GencodeFuncotationFactory.java:499)
22 Jun 2023 14:54:27,163 DEBUG: 		at org.broadinstitute.hellbender.tools.funcotator.DataSourceFuncotationFactory.createFuncotations(DataSourceFuncotationFactory.java:217)
22 Jun 2023 14:54:27,164 DEBUG: 		at org.broadinstitute.hellbender.tools.funcotator.DataSourceFuncotationFactory.createFuncotations(DataSourceFuncotationFactory.java:182)
22 Jun 2023 14:54:27,166 DEBUG: 		at org.broadinstitute.hellbender.tools.funcotator.FuncotatorEngine.lambda$createFuncotationMapForVariant$0(FuncotatorEngine.java:152)
22 Jun 2023 14:54:27,167 DEBUG: 		at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
22 Jun 2023 14:54:27,168 DEBUG: 		at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)
22 Jun 2023 14:54:27,170 DEBUG: 		at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1625)
22 Jun 2023 14:54:27,171 DEBUG: 		at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
22 Jun 2023 14:54:27,172 DEBUG: 		at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
22 Jun 2023 14:54:27,174 DEBUG: 		at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
22 Jun 2023 14:54:27,175 DEBUG: 		at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
22 Jun 2023 14:54:27,177 DEBUG: 		at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
22 Jun 2023 14:54:27,178 DEBUG: 		at org.broadinstitute.hellbender.tools.funcotator.FuncotatorEngine.createFuncotationMapForVariant(FuncotatorEngine.java:162)
22 Jun 2023 14:54:27,180 DEBUG: 		at com.github.discvrseq.walkers.ExtendedFuncotator.enqueueAndHandleVariant(ExtendedFuncotator.java:209)
22 Jun 2023 14:54:27,181 DEBUG: 		at org.broadinstitute.hellbender.tools.funcotator.Funcotator.apply(Funcotator.java:878)

bbimber · 2023-06-26T17:01:53Z

@droazen or @jonn-smith: checking back: any chance someone would be able to approve the workflow so tests can run?

jonn-smith · 2023-06-26T17:06:35Z

@bbimber - Sorry for the delay approved and running.

bbimber · 2023-06-26T19:48:40Z

@jonn-smith: thanks. looks like the tests passed. what do you think about the PR and level of testing?

bbimber · 2023-07-05T21:41:38Z

@droazen and/or @jonn-smith: is there anything else you need on this PR? It appears the test is passing

droazen

@bbimber Back to you with a quick request

droazen · 2023-07-07T16:41:26Z

...tute/hellbender/tools/funcotator/dataSources/xsv/LocatableXsvFuncotationFactoryUnitTest.java

+        Object[][] arr1 = provideDataForTestCreateFuncotations(false);
+        Object[][] arr2 = provideDataForTestCreateFuncotations(true);
+        return Stream.concat(Arrays.stream(arr1), Arrays.stream(arr2)).
+                toArray(size -> (Object[][]) Array.newInstance(arr1.getClass().getComponentType(), arr1.length + arr2.length));


Could you use the preexisting utility method Utils.concat(arr1, arr2, Object[][]::new) here?

bbimber · 2023-07-07T16:50:37Z

@droazen: the change is added. it looks like this needs approval to kick off tests?

droazen

👍 Tests pass, merging this one

bbimber · 2023-07-10T18:49:36Z

@droazen Thanks!

Allow LocatableXsvFuncotationFactory to read gzipped files

85d07fe

droazen suggested changes Jun 22, 2023

View reviewed changes

Add test case

f717afb

Dont allow negative start position

d8f2d89

bbimber requested a review from droazen June 30, 2023 19:27

droazen suggested changes Jul 7, 2023

View reviewed changes

droazen self-assigned this Jul 7, 2023

Code review

2e2f783

droazen approved these changes Jul 10, 2023

View reviewed changes

droazen merged commit 810326c into broadinstitute:master Jul 10, 2023

bbimber deleted the locatableXSV branch July 10, 2023 18:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow LocatableXsvFuncotationFactory to read gzipped files #8363

Allow LocatableXsvFuncotationFactory to read gzipped files #8363

bbimber commented Jun 14, 2023

bbimber commented Jun 14, 2023

jonn-smith commented Jun 14, 2023

bbimber commented Jun 14, 2023

bbimber commented Jun 14, 2023

bbimber commented Jun 21, 2023

droazen left a comment

bbimber commented Jun 22, 2023

bbimber commented Jun 22, 2023

bbimber commented Jun 22, 2023

bbimber commented Jun 26, 2023

jonn-smith commented Jun 26, 2023

bbimber commented Jun 26, 2023

bbimber commented Jul 5, 2023

droazen left a comment

droazen Jul 7, 2023

bbimber Jul 7, 2023

bbimber commented Jul 7, 2023

droazen left a comment

bbimber commented Jul 10, 2023

Allow LocatableXsvFuncotationFactory to read gzipped files #8363

Allow LocatableXsvFuncotationFactory to read gzipped files #8363

Conversation

bbimber commented Jun 14, 2023

bbimber commented Jun 14, 2023

jonn-smith commented Jun 14, 2023

bbimber commented Jun 14, 2023

bbimber commented Jun 14, 2023

bbimber commented Jun 21, 2023

droazen left a comment

Choose a reason for hiding this comment

bbimber commented Jun 22, 2023

bbimber commented Jun 22, 2023

bbimber commented Jun 22, 2023

bbimber commented Jun 26, 2023

jonn-smith commented Jun 26, 2023

bbimber commented Jun 26, 2023

bbimber commented Jul 5, 2023

droazen left a comment

Choose a reason for hiding this comment

droazen Jul 7, 2023

Choose a reason for hiding this comment

bbimber Jul 7, 2023

Choose a reason for hiding this comment

bbimber commented Jul 7, 2023

droazen left a comment

Choose a reason for hiding this comment

bbimber commented Jul 10, 2023