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

Updated datasource version parsing #5149

Merged
merged 2 commits into from
Aug 31, 2018

Conversation

jonn-smith
Copy link
Collaborator

Now looks at manifest file, not readme.
Now supports version decorators (e.g. somatic) after version numbers in manifest file.
Added in a unit test to check the version regex.

Fixes #4582
Fixes #4692

Added in a unit test to check the version regex.
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 Minor stuff... feel free to merge when addressed.

@@ -461,12 +462,14 @@ private static Properties readConfigFileProperties(final Path configFilePath) {
* user can create their own data sources directory, which may not contain the metadata we seek.
*
* NOTE: The README file in a Data Sources directory is assumed to have the following properties:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this now read: The MANIFEST file in a Data ....?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. Fixed!


//==================================================================================================================
// Public Static Members:

/** The minimum version of the data sources required for funcotator to run. */
public static final String CURRENT_MINIMUM_DATA_SOURCE_VERSION = String.format("v%d.%d.%d%02d%02d", MIN_MAJOR_VERSION_NUMBER, MIN_MINOR_VERSION_NUMBER, MIN_YEAR_RELEASED, MIN_MONTH_RELEASED, MIN_DAY_RELEASED);
public static final String README_FILE_NAME = "README.txt";
public static final String MANIFEST_FILE_NAME = "MANIFEST.txt";
Copy link
Collaborator

@droazen droazen Aug 31, 2018

Choose a reason for hiding this comment

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

Did you just rename README.txt to MANIFEST.txt, or is this a more substantial change? Ie., is MANIFEST.txt now a file with a well-defined structured format, or is it still basically a textual README with another name?

Copy link
Collaborator Author

@jonn-smith jonn-smith Aug 31, 2018

Choose a reason for hiding this comment

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

@droazen I added a MANIFEST.txt file to the packages. MANIFEST.txt contains the version and provenance information for each data source package, as well as a new decorator/information field to distinguish somatic and germline files. This version information is duplicated in the readme, but the readme is not parsed.

MANIFEST.txt a plain-text formatted file with 1 field per line. The formatting is loosely enforced with the regex in DataSourceUtils and is of the syntax:

<FIELD>: <VALUE>

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, sounds good @jonn-smith, thanks for the clarification.

@droazen droazen assigned jonn-smith and unassigned LeeTL1220 Aug 31, 2018
@codecov-io
Copy link

Codecov Report

Merging #5149 into master will increase coverage by 0.03%.
The diff coverage is 55.556%.

@@              Coverage Diff               @@
##              master     #5149      +/-   ##
==============================================
+ Coverage     86.733%   86.763%   +0.03%     
- Complexity     29305     29450     +145     
==============================================
  Files           1810      1813       +3     
  Lines         135527    136056     +529     
  Branches       15031     15140     +109     
==============================================
+ Hits          117547    118046     +499     
- Misses         12566     12575       +9     
- Partials        5414      5435      +21
Impacted Files Coverage Δ Complexity Δ
...uncotator/dataSources/DataSourceUtilsUnitTest.java 100% <100%> (ø) 15 <8> (+8) ⬆️
.../tools/funcotator/dataSources/DataSourceUtils.java 62.257% <22.222%> (-0.488%) 40 <0> (ø)
...r/tools/walkers/mutect/Mutect2IntegrationTest.java 91.429% <0%> (-0.428%) 63% <0%> (+3%)
...rs/variantutils/SelectVariantsIntegrationTest.java 100% <0%> (ø) 85% <0%> (+19%) ⬆️
...dorientation/CollectF1R2CountsIntegrationTest.java 100% <0%> (ø) 18% <0%> (+2%) ⬆️
...lbender/utils/haplotype/AllHaplotypeBAMWriter.java 100% <0%> (ø) 4% <0%> (?)
...ellbender/tools/walkers/mutect/M2TestingUtils.java 91.304% <0%> (ø) 6% <0%> (?)
...nder/utils/haplotype/CalledHaplotypeBAMWriter.java 100% <0%> (ø) 5% <0%> (?)
...er/utils/haplotype/HaplotypeBAMWriterUnitTest.java 90.104% <0%> (+1.029%) 35% <0%> (+12%) ⬆️
...der/tools/walkers/variantutils/SelectVariants.java 81.075% <0%> (+1.489%) 178% <0%> (+63%) ⬆️
... and 4 more

@jonn-smith jonn-smith merged commit 006b1b9 into master Aug 31, 2018
@jonn-smith jonn-smith deleted the jts_funcotator_datasource_refactoring_4582_4692 branch August 31, 2018 17:43
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