Skip to content
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

Eliminated the filtering by user transcripts and fixed locus level issue #4931

Merged
merged 1 commit into from
Jun 22, 2018

Conversation

LeeTL1220
Copy link
Contributor

@LeeTL1220 LeeTL1220 commented Jun 21, 2018

@codecov-io
Copy link

codecov-io commented Jun 21, 2018

Codecov Report

Merging #4931 into master will increase coverage by 0.003%.
The diff coverage is 87.5%.

@@               Coverage Diff               @@
##              master     #4931       +/-   ##
===============================================
+ Coverage     80.471%   80.474%   +0.003%     
+ Complexity     17855     17854        -1     
===============================================
  Files           1093      1093               
  Lines          64295     64295               
  Branches       10378     10378               
===============================================
+ Hits           51739     51741        +2     
+ Misses          8501      8500        -1     
+ Partials        4055      4054        -1
Impacted Files Coverage Δ Complexity Δ
...dataSources/gencode/GencodeFuncotationFactory.java 83.308% <ø> (-0.325%) 160 <0> (-4)
...bender/utils/codecs/gencode/GencodeGtfFeature.java 78.453% <ø> (ø) 44 <0> (ø) ⬇️
...e/hellbender/tools/funcotator/FuncotatorUtils.java 80.357% <100%> (+0.027%) 169 <1> (+1) ⬆️
...nder/tools/funcotator/TranscriptSelectionMode.java 89.72% <83.333%> (ø) 1 <0> (ø) ⬇️
...utils/smithwaterman/SmithWatermanIntelAligner.java 90% <0%> (+40%) 3% <0%> (+2%) ⬆️

…priority order. The filtering step has been eliminated. Fixes #4918

- Fixed issue where locus level ranking was being reversed.  Updated tests.  This was identified thanks to the thousands of tests in Funcotator (only one failed).
@LeeTL1220 LeeTL1220 force-pushed the ll_no_user_tx_filter_only_priority branch from 1cf9fd7 to 8e00e3a Compare June 22, 2018 18:50
Copy link
Collaborator

@jonn-smith jonn-smith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@jonn-smith
Copy link
Collaborator

Once tests pass, feel free to merge.

@LeeTL1220
Copy link
Contributor Author

LeeTL1220 commented Jun 22, 2018

Test failure is false alarm? @cmnbroad

@LeeTL1220
Copy link
Contributor Author

LeeTL1220 commented Jun 22, 2018

@cmnbroad I am going to restart and see if it goes away. I am only bugging you about it since it is the python test. But my PR goes nowhere near that code.

@cmnbroad
Copy link
Collaborator

@LeeTL1220 Sorry - I didn't see the before it was restarted so I'm unsure which test failed. Looks like its nearly finished.

@LeeTL1220
Copy link
Contributor Author

LeeTL1220 commented Jun 22, 2018 via email

@LeeTL1220
Copy link
Contributor Author

Assuming failed push test is transient, since this PR does not affect the python env.

@LeeTL1220 LeeTL1220 merged commit ee7c717 into master Jun 22, 2018
@LeeTL1220 LeeTL1220 deleted the ll_no_user_tx_filter_only_priority branch June 22, 2018 21:02
danxmoran pushed a commit that referenced this pull request Jun 26, 2018
…priority order. The filtering step has been eliminated. Fixes #4918 (#4931)

- User defined transcripts were being used as a filter rather than a priority order.  The filtering step has been eliminated.  Closes #4918 
- Fixed previously unidentified issue where locus level ranking was being reversed.  Updated tests.  This was identified thanks to the thousands of tests in Funcotator (only one failed, but that was all it took).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants