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

Changing the name of the GencodeFuncotationFactory to be read from file. #4823

Merged
merged 1 commit into from May 30, 2018

Conversation

jonn-smith
Copy link
Collaborator

Fixes #3956

Now gencode data sources have names preserved from config files.
Updated MafOutputRenderer to put a space and delimiter between the date and first funcotation factory information.
Updated some test cases to be correct with the new Gencode name preservation and MAF renderer update.

Copy link
Contributor

@LeeTL1220 LeeTL1220 left a comment

Choose a reason for hiding this comment

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

@jonn-smith Back to you... minor stuff.

@@ -203,24 +203,24 @@ private static void addManualAnnotationsToArguments(final ArgumentsBuilder argum
true,
FuncotatorTestConstants.REFERENCE_VERSION_HG19,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove commented code or file an issue that it should be uncommented and when?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It should not be commented out - I just wanted to disable some of the tests. Fixed.

@@ -165,26 +175,27 @@
// Constructors:

public GencodeFuncotationFactory(final Path gencodeTranscriptFastaFile, final String version) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If the constructors use DEFAULT_NAME doesn't this mean that the config file will be ignored? Or is the constructor with the name parameter the only one that is used? Can you delete some of the other constructors?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The constructor that was being called in the code was the one that took a name. I deleted the other constructors for clarity as suggested.

Fixes #3956

Now gencode data sources have names preserved from config files.

Updated MafOutputRenderer to put a space and delimiter between the date and first funcotation factory information.
Updated some test cases to be correct with the new Gencode name preservation and MAF renderer update.

Addressed code review comments.
@jonn-smith jonn-smith force-pushed the jts_mutltiple_gtf_file_support_3956 branch from 3691eec to c7bd47b Compare May 29, 2018 19:51
@codecov-io
Copy link

Codecov Report

Merging #4823 into master will decrease coverage by 1.819%.
The diff coverage is 87.234%.

@@               Coverage Diff               @@
##              master     #4823       +/-   ##
===============================================
- Coverage     80.332%   78.513%   -1.819%     
+ Complexity     17625     17444      -181     
===============================================
  Files           1088      1088               
  Lines          63849     64601      +752     
  Branches       10276     10475      +199     
===============================================
- Hits           51291     50720      -571     
- Misses          8558      9877     +1319     
- Partials        4000      4004        +4
Impacted Files Coverage Δ Complexity Δ
.../tools/funcotator/mafOutput/MafOutputRenderer.java 95.291% <100%> (+0.013%) 48 <0> (ø) ⬇️
...dataSources/gencode/GencodeFuncotationBuilder.java 96.721% <100%> (+0.111%) 30 <1> (+1) ⬆️
.../tools/funcotator/dataSources/DataSourceUtils.java 63.241% <100%> (+0.146%) 40 <0> (ø) ⬇️
...dataSources/gencode/GencodeFuncotationFactory.java 78.091% <81.818%> (-4.413%) 152 <1> (ø)
...otator/dataSources/gencode/GencodeFuncotation.java 55.862% <87.5%> (+2.545%) 194 <112> (+2) ⬆️
...s/GermlineContigPloidyModelArgumentCollection.java 0% <0%> (-100%) 0% <0%> (-4%)
...er/utils/python/PythonScriptExecutorException.java 0% <0%> (-100%) 0% <0%> (-1%)
...r/arguments/GermlineCallingArgumentCollection.java 0% <0%> (-100%) 0% <0%> (-4%)
...mlineContigPloidyHybridADVIArgumentCollection.java 0% <0%> (-100%) 0% <0%> (-3%)
...ments/GermlineCNVHybridADVIArgumentCollection.java 0% <0%> (-100%) 0% <0%> (-3%)
... and 32 more

@jonn-smith jonn-smith merged commit 75a79ba into master May 30, 2018
@jonn-smith jonn-smith deleted the jts_mutltiple_gtf_file_support_3956 branch May 30, 2018 16:00
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.

None yet

3 participants