-
Notifications
You must be signed in to change notification settings - Fork 594
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
Now non-locatable data sources can create funcotations again. #5774
Conversation
Primarily the large integration tests check for the non-locatable funcotation factories producing data. Regenerated the expected output from large tests. Fixes #5773
Codecov Report
@@ Coverage Diff @@
## master #5774 +/- ##
===============================================
+ Coverage 86.985% 86.998% +0.013%
- Complexity 31863 31871 +8
===============================================
Files 1943 1942 -1
Lines 146768 146789 +21
Branches 16223 16216 -7
===============================================
+ Hits 127666 127703 +37
+ Misses 13189 13177 -12
+ Partials 5913 5909 -4
|
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.
Minor stuff and a question. If you answer, feel free to merge.
@@ -292,7 +292,7 @@ if [[ $r -eq 0 ]] && ${doRunLargeTests} ; then | |||
#INPUT=/Users/jonn/Development/gatk/src/test/resources/large/funcotator/regressionTestVariantSet2.vcf | |||
#INPUT=/Users/jonn/Development/gatk/src/test/resources/large/funcotator/regressionTestHg19Large.vcf | |||
#INPUT=/Users/jonn/Development/gatk/hg38_trio_liftoverb37.vcf | |||
#INPUT=/Users/jonn/Development/gatk/tmp.vcf | |||
INPUT=/Users/jonn/Development/gatk/tmp.vcf |
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.
Not sure why this is needed. What is this stuff?
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.
Left over from testing. I'll remove.
@@ -135,6 +135,12 @@ public String getVersion() { | |||
return version; | |||
} | |||
|
|||
/** | |||
* @return {@code True} if this {@link DataSourceFuncotationFactory} requires features to create {@link Funcotation}s. {@code False} otherwise. |
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.
Any features? Or specific types? Please update docs.
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.
Yes - any features. If the DataSourceFuncotationFactory relies on feature matching that the GATK engine provides, then this should be true.
Things that don't require features (like gene name matching data sources) don't require any features to be present to create a funcotation.
|
||
final List<Funcotation> outputFuncotations; | ||
// Only get features if we need them: |
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.
Can you collapse to one line? blah? truestuff: falsestuff
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.
Sure.
Added in another specific test for non-locatable data sources.
Comments addressed. I also added in another test to check for this specific case and guard against it in the future. |
Primarily the large integration tests check for the non-locatable
funcotation factories producing data.
Regenerated the expected output from large tests.
Fixes #5773