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

Remove unused classes following #5127. #5292

Merged
merged 1 commit into from
Jan 11, 2019

Conversation

tomwhite
Copy link
Contributor

@tomwhite tomwhite commented Oct 8, 2018

  • AddContextDataToReadSpark (and AddContextDataToReadSparkUnitTest) implemented the different JoinStrategy options for
    BQSR; has been replaced with the Spark Files mecahnism (see Use SparkFiles to localize reference and known sites #5127)
  • BroadcastJoinReadsWithRefBases and JoinReadsWithRefBasesSparkUnitTest were only used by AddContextDataToReadSpark
  • BroadcastJoinReadsWithVariants and JoinReadsWithVariantsSparkUnitTest were only used by AddContextDataToReadSpark
  • ShuffleJoinReadsWithRefBases and ShuffleJoinReadsWithVariants were only used by AddContextDataToReadSpark
  • JoinStrategy was only used for BQSR (HC always uses overlaps partitioner), but is no longer used since Use SparkFiles to localize reference and known sites #5127
  • KnownSitesCache was replaced with Spark Files
  • ReferenceMultiSourceAdapter in HaplotypeCallerSpark was replaced with the regular ReferenceDataSource
  • BaseRecalibratorEngineSparkWrapper was only used by BaseRecalibratorSparkSharded, which was removed in Move/rename some stray Spark datasource classes, and delete BaseRecalibratorSparkSharded #5192

@codecov-io
Copy link

codecov-io commented Oct 8, 2018

Codecov Report

Merging #5292 into master will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #5292      +/-   ##
============================================
- Coverage     87.08%   87.04%   -0.05%     
+ Complexity    31522    31436      -86     
============================================
  Files          1930     1919      -11     
  Lines        145236   144942     -294     
  Branches      16096    16059      -37     
============================================
- Hits         126480   126163     -317     
- Misses        12903    12936      +33     
+ Partials       5853     5843      -10
Impacted Files Coverage Δ Complexity Δ
...der/tools/HaplotypeCallerSparkIntegrationTest.java 68.29% <ø> (ø) 16 <0> (ø) ⬇️
...stitute/hellbender/tools/HaplotypeCallerSpark.java 82.75% <ø> (ø) 20 <0> (ø) ⬇️
...spark/ReadsPreprocessingPipelineSparkTestData.java 0% <0%> (-94.03%) 0% <0%> (-11%)
...adinstitute/hellbender/engine/ReadContextData.java 0% <0%> (-70.38%) 0% <0%> (-6%)
...n/java/org/broadinstitute/hellbender/utils/KV.java 0% <0%> (-57.15%) 0% <0%> (-5%)
...ute/hellbender/utils/reference/ReferenceBases.java 38.46% <0%> (-19.24%) 4% <0%> (-2%)
...oadinstitute/hellbender/engine/ReferenceShard.java 62.5% <0%> (-18.75%) 6% <0%> (-1%)
...broadinstitute/hellbender/engine/VariantShard.java 63.63% <0%> (-13.64%) 7% <0%> (-1%)
...ne/spark/datasources/ReferenceWindowFunctions.java 12.5% <0%> (-12.5%) 1% <0%> (ø)
...oadinstitute/hellbender/utils/gcs/BucketUtils.java 79.87% <0%> (+1.21%) 42% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a74e571...3768ba2. Read the comment docs.

Copy link
Collaborator

@jamesemery jamesemery left a comment

Choose a reason for hiding this comment

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

I'm not sure that we won't ever decide we want these classes in the future, which is why I was hesitant to approve this commit. However we will always be able to refer back to these PR closings if we need to resurrect this code in the future.

* AddContextDataToReadSpark (and AddContextDataToReadSparkUnitTest) implemented the different JoinStrategy options for
  BQSR; has been replaced with the Spark Files mecahnism (see #5127)
* BroadcastJoinReadsWithRefBases and JoinReadsWithRefBasesSparkUnitTest were only used by AddContextDataToReadSpark
* BroadcastJoinReadsWithVariants and JoinReadsWithVariantsSparkUnitTest were only used by AddContextDataToReadSpark
* ShuffleJoinReadsWithRefBases and ShuffleJoinReadsWithVariants were only used by AddContextDataToReadSpark
* JoinStrategy was only used for BQSR (HC always uses overlaps partitioner), but is no longer used since #5127
* KnownSitesCache was replaced with Spark Files
* ReferenceMultiSourceAdapter in HaplotypeCallerSpark was replaced with the regular ReferenceDataSource
* BaseRecalibratorEngineSparkWrapper was only used by BaseRecalibratorSparkSharded, which was removed in #5192
@jamesemery jamesemery force-pushed the tw_spark_files_optimization_cleanup branch from bd0247d to 3768ba2 Compare January 11, 2019 19:27
@jamesemery jamesemery merged commit b50e9cc into master Jan 11, 2019
@jamesemery jamesemery deleted the tw_spark_files_optimization_cleanup branch January 11, 2019 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants