Funcotator - removing the required transcript file #8863

jonn-smith · 2024-06-05T14:37:54Z

This PR removes the requirement for Gencode datasources to have transcript files. This is the first step in fixing some long-standing funcotator issues involving variants that run over the edge of transcripts.

Next step is to remove the required transcript files.

gatk-bot · 2024-06-05T15:00:33Z

Github actions tests reported job failures from actions build 9386157234
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
unit	17.0.6+10	9386157234.12	logs
integration	17.0.6+10	9386157234.11	logs
unit	17.0.6+10	9386157234.1	logs
integration	17.0.6+10	9386157234.0	logs

gatk-bot · 2024-06-11T18:12:05Z

Github actions tests reported job failures from actions build 9470383170
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
unit	17.0.6+10	9470383170.12	logs
integration	17.0.6+10	9470383170.11	logs
unit	17.0.6+10	9470383170.1	logs
integration	17.0.6+10	9470383170.0	logs

gatk-bot · 2024-06-26T20:15:06Z

Github actions tests reported job failures from actions build 9685898135
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
unit	17.0.6+10	9685898135.12	logs
integration	17.0.6+10	9685898135.11	logs
unit	17.0.6+10	9685898135.1	logs
integration	17.0.6+10	9685898135.0	logs

gatk-bot · 2024-06-26T23:38:38Z

Github actions tests reported job failures from actions build 9688050367
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
unit	17.0.6+10	9688050367.12	logs
integration	17.0.6+10	9688050367.11	logs
unit	17.0.6+10	9688050367.1	logs
integration	17.0.6+10	9688050367.0	logs

gatk-bot · 2024-06-27T01:02:24Z

Github actions tests reported job failures from actions build 9688780153
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
unit	17.0.6+10	9688780153.12	logs
integration	17.0.6+10	9688780153.11	logs
unit	17.0.6+10	9688780153.1	logs
integration	17.0.6+10	9688780153.0	logs

gatk-bot · 2024-06-27T04:52:56Z

Github actions tests reported job failures from actions build 9690704227
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
unit	17.0.6+10	9690704227.12	logs
integration	17.0.6+10	9690704227.11	logs
unit	17.0.6+10	9690704227.1	logs
integration	17.0.6+10	9690704227.0	logs

gatk-bot · 2024-06-27T05:31:44Z

Github actions tests reported job failures from actions build 9690907019
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
integration	17.0.6+10	9690907019.11	logs
integration	17.0.6+10	9690907019.0	logs

gatk-bot · 2024-06-27T16:34:34Z

Github actions tests reported job failures from actions build 9699861398
Failures in the following jobs:

Test Type	JDK	Job ID	Logs
integration	17.0.6+10	9699861398.11	logs
integration	17.0.6+10	9699861398.0	logs

jamesemery · 2024-10-01T15:02:14Z

@jonn-smith Looks like that did the trick. The tests are passing now. How do you want to handle this branch? should I give you a review or try to take it over and let you review the changes i make to it?

src/main/java/org/broadinstitute/hellbender/utils/TranscriptUtils.java

jamesemery

Some clean-up requests. A few questions about assumptions. One maybe bug/oversight. A suggestion for a stronger test that would put to bed any lingering questions about this branch I think.

src/main/java/org/broadinstitute/hellbender/tools/funcotator/FuncotatorEngine.java

jamesemery · 2024-10-02T14:43:45Z

...roadinstitute/hellbender/tools/funcotator/dataSources/gencode/GencodeFuncotationFactory.java

+
+        // If we're on the reverse strand, we need to reverse complement the sequence:
+        if ( transcript.getGenomicStrand() == Strand.NEGATIVE ) {
+            return new String(BaseUtils.simpleReverseComplement(TranscriptUtils.extractTrascriptFromReference(referenceContext, transcriptFeatureList, doExonContigConversionToB37ForTranscripts).getBytes())) + tailPaddingBases;


These tail padding bases are not being RCed after you reverse complement the expression bases... that seems incorrect and like it could lead to some nasty bug down the line

...roadinstitute/hellbender/tools/funcotator/dataSources/gencode/GencodeFuncotationFactory.java

jamesemery · 2024-10-02T14:47:43Z

...roadinstitute/hellbender/tools/funcotator/dataSources/gencode/GencodeFuncotationFactory.java

+        // Finally, if we're on the reverse strand, we need to reverse complement the UTR bases.
+        // NOTE: the extra bases are not reverse complemented because they are not part of the UTR.
+        if (transcript.getGenomicStrand() == Strand.NEGATIVE) {
+            return new String(BaseUtils.simpleReverseComplement(utrBases.getBytes())) + new String(BaseUtils.simpleReverseComplement(extraBases.getBytes()));


is it true that the extra bases go AFTER the sequence here? Looks like yes?

...roadinstitute/hellbender/tools/funcotator/dataSources/gencode/GencodeFuncotationFactory.java

src/main/java/org/broadinstitute/hellbender/utils/TranscriptUtils.java

...itute/hellbender/tools/funcotator/dataSources/gencode/GencodeFuncotationFactoryUnitTest.java

src/test/java/org/broadinstitute/hellbender/utils/TranscriptUtilsUnitTest.java

jamesemery · 2024-10-02T15:04:35Z

src/test/java/org/broadinstitute/hellbender/utils/TranscriptUtilsUnitTest.java

+    private static final ReferenceDataSource refDataSourceHg19Ch3;
+    private static final ReferenceDataSource refDataSourceB37;
+
+    private static final List<AutoCloseable> autoCloseableList = new ArrayList<>();


these tests are reasonable... however they missed the off by 1 bug that was one layer up from this method grabbing the transcrit bases that is not exposed (and possibly not as easy to test because it involves the transcript datasources directly)

jamesemery · 2024-10-02T15:06:14Z

src/test/java/org/broadinstitute/hellbender/utils/TranscriptUtilsUnitTest.java

+    private static final List<AutoCloseable> autoCloseableList = new ArrayList<>();
+    static {
+        refDataSourceHg19Ch3 = ReferenceDataSource.of( IOUtils.getPath(FuncotatorReferenceTestUtils.retrieveHg19Chr3Ref()) );
+        refDataSourceB37 = ReferenceDataSource.of( IOUtils.getPath(b37Reference) );


How difficult would it be to check in a chunk of datasource that HAS these transcript files in it and just call the get___fromReference() methods and directly do the string comparison with the transcript file that is checked in from gencode directly? That seems like the strongest possible test and not too terribly difficult to write

Good idea. I think we should have this available for testing already. I can add another test to do this.

jonn-smith added 3 commits June 4, 2024 12:21

Added in TranscriptUtils

d17ff2e

Added tests.

831ff6a

Changed code to pull coding sequence from reference.

1ad11df

Next step is to remove the required transcript files.

Revised methods for pulling transcripts from the reference.

2566501

Fixing issue that had to do with b37/hg19 conversion.

da656cd

Fixing bug.

a537762

Fixed another spot with HG19 / B37 conversion.

adb2efe

Found another spot for B37 / HG19 conversion.

3dddea3

Fixed another b37 / HG19 mismatch.

606312a

Fixed one test.

f0aa453

obob patch

e6b0d5b

jamesemery reviewed Oct 2, 2024

View reviewed changes

src/main/java/org/broadinstitute/hellbender/utils/TranscriptUtils.java Show resolved Hide resolved

jamesemery requested changes Oct 2, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Funcotator - removing the required transcript file #8863

Funcotator - removing the required transcript file #8863

jonn-smith commented Jun 5, 2024

gatk-bot commented Jun 5, 2024 •

edited

Loading

gatk-bot commented Jun 11, 2024 •

edited

Loading

gatk-bot commented Jun 26, 2024 •

edited

Loading

gatk-bot commented Jun 26, 2024 •

edited

Loading

gatk-bot commented Jun 27, 2024 •

edited

Loading

gatk-bot commented Jun 27, 2024 •

edited

Loading

gatk-bot commented Jun 27, 2024 •

edited

Loading

gatk-bot commented Jun 27, 2024 •

edited

Loading

jamesemery commented Oct 1, 2024

jamesemery left a comment

jamesemery Oct 2, 2024

jamesemery Oct 2, 2024

jamesemery Oct 2, 2024

jamesemery Oct 2, 2024

jonn-smith Oct 18, 2024

Funcotator - removing the required transcript file #8863

Are you sure you want to change the base?

Funcotator - removing the required transcript file #8863

Conversation

jonn-smith commented Jun 5, 2024

gatk-bot commented Jun 5, 2024 • edited Loading

gatk-bot commented Jun 11, 2024 • edited Loading

gatk-bot commented Jun 26, 2024 • edited Loading

gatk-bot commented Jun 26, 2024 • edited Loading

gatk-bot commented Jun 27, 2024 • edited Loading

gatk-bot commented Jun 27, 2024 • edited Loading

gatk-bot commented Jun 27, 2024 • edited Loading

gatk-bot commented Jun 27, 2024 • edited Loading

jamesemery commented Oct 1, 2024

jamesemery left a comment

Choose a reason for hiding this comment

jamesemery Oct 2, 2024

Choose a reason for hiding this comment

jamesemery Oct 2, 2024

Choose a reason for hiding this comment

jamesemery Oct 2, 2024

Choose a reason for hiding this comment

jamesemery Oct 2, 2024

Choose a reason for hiding this comment

jonn-smith Oct 18, 2024

Choose a reason for hiding this comment

gatk-bot commented Jun 5, 2024 •

edited

Loading

gatk-bot commented Jun 11, 2024 •

edited

Loading

gatk-bot commented Jun 26, 2024 •

edited

Loading

gatk-bot commented Jun 26, 2024 •

edited

Loading

gatk-bot commented Jun 27, 2024 •

edited

Loading

gatk-bot commented Jun 27, 2024 •

edited

Loading

gatk-bot commented Jun 27, 2024 •

edited

Loading

gatk-bot commented Jun 27, 2024 •

edited

Loading