Finishing up the cleanup on org.bdgenomics.adam.rdd. #1264

Merged
merged 1 commit into from Nov 16, 2016

Conversation

Projects
None yet
3 participants
@fnothaft
Member

fnothaft commented Nov 13, 2016

  • Made MDTagging private to read. Exposed it through AlignmentRecordRDD.
    Cleaned up where it was used in Transform.
  • Made FlagStat and most related models private to read.
  • Made all read.recalibration and read.realignment classes private to
    package. Depending on the class, this varied from:
    • adam for classes that need to be registered with kryo
    • read for the core RealignIndels and BaseQualityRecalibrator engines
    • Their subpackage where possible.
  • Made class values for genomic partitioners as private as possible.
  • Added documentation to all InFormatters, InFormatterCompanions, and OutFormatters. Made the InFormatters package private with private constructors.
  • Added method/class level scaladoc where missing.
  • Moved org.bdgenomics.adam.cli.FlagStatSuite to org.bdgenomics.adam.read, along with the NA12878.sam test resource, which was otherwise unused in the adam-cli submodule.
@heuermh

This comment has been minimized.

Show comment
Hide comment
@heuermh

heuermh Nov 13, 2016

Member

I may have asked elsewhere, it seems like the InFormatters and OutFormatters need to be publicly accessible, i.e.

https://github.com/heuermh/adam-snpeff/blob/master/src/main/scala/com/github/heuermh/adam/snpeff/AdamSnpEff.scala#L52

It may be that I'm not using them correctly here, in that they aren't really implicit if I have to explicitly instantiate them.

Member

heuermh commented Nov 13, 2016

I may have asked elsewhere, it seems like the InFormatters and OutFormatters need to be publicly accessible, i.e.

https://github.com/heuermh/adam-snpeff/blob/master/src/main/scala/com/github/heuermh/adam/snpeff/AdamSnpEff.scala#L52

It may be that I'm not using them correctly here, in that they aren't really implicit if I have to explicitly instantiate them.

@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Nov 13, 2016

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1595/
Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1595/
Test PASSed.

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Nov 13, 2016

Member

The InFormatterCompanion and OutFormatter need to be public. The InFormatter has to be accessible from rdd, but it's constructor only has to be accessible from the companion.

Member

fnothaft commented Nov 13, 2016

The InFormatterCompanion and OutFormatter need to be public. The InFormatter has to be accessible from rdd, but it's constructor only has to be accessible from the companion.

@heuermh

This comment has been minimized.

Show comment
Hide comment
@heuermh

heuermh Nov 14, 2016

Member

Thanks for the clarification. I'll take a closer look at the formatter docs and the rest of this pull request tomorrow morning.

Member

heuermh commented Nov 14, 2016

Thanks for the clarification. I'll take a closer look at the formatter docs and the rest of this pull request tomorrow morning.

@heuermh

Looks good, thanks!

fnothaft added a commit to fnothaft/adam that referenced this pull request Nov 15, 2016

[ADAM-1083] Cleaning up `org.bdgenomics.adam.models`.
Along with #1263 and #1264, this resolves #1083.

* Removing unused org.bdgenomics.adam.models.ReadBucket class.
* Move org.bdgenomics.adam.models.ReferencePositionPair and
  org.bdgenomics.adam.models.SingleReadBucket in to org.bdgenomics.adam.rdd.read
  and make package private.
* Clean up duplicated methods and methods that were incorrectly in companion
  singleton for SequenceDictionary and ReadGroupDictionary.
* Removed all SamReader references.
* Make writable file headers private to ADAM.
* Eliminated manual VCF parsing code in SnpTable.
* Cleaned up scaladoc for all classes and singleton objects.
* Moved `NonoverlappingRegions` test code out of `InnerBroadcastRegionJoinSuite`.

fnothaft added a commit to fnothaft/adam that referenced this pull request Nov 15, 2016

[ADAM-1083] Cleaning up `org.bdgenomics.adam.models`.
Along with #1263 and #1264, this resolves #1083.

* Removing unused org.bdgenomics.adam.models.ReadBucket class.
* Move org.bdgenomics.adam.models.ReferencePositionPair and
  org.bdgenomics.adam.models.SingleReadBucket in to org.bdgenomics.adam.rdd.read
  and make package private.
* Clean up duplicated methods and methods that were incorrectly in companion
  singleton for SequenceDictionary and ReadGroupDictionary.
* Removed all SamReader references.
* Make writable file headers private to ADAM.
* Eliminated manual VCF parsing code in SnpTable.
* Cleaned up scaladoc for all classes and singleton objects.
* Moved `NonoverlappingRegions` test code out of `InnerBroadcastRegionJoinSuite`.
Finishing up the cleanup on org.bdgenomics.adam.rdd.
* Made `MDTagging` private to `read`. Exposed it through `AlignmentRecordRDD`.
  Cleaned up where it was used in `Transform`.
* Made `FlagStat` and most related models private to `read`.
* Made all `read.recalibration` and `read.realignment` classes private to
  package. Depending on the class, this varied from:
  * `adam` for classes that need to be registered with `kryo`
  * `read` for the core `RealignIndels` and `BaseQualityRecalibrator` engines
  * Their subpackage where possible.
* Made class values for genomic partitioners as private as possible.
* Added documentation to all `InFormatter`s, `InFormatterCompanion`s, and
  `OutFormatter`s. Made the `InFormatter`s package private with private
  constructors.
* Added method/class level scaladoc where missing.
* Moved `org.bdgenomics.adam.cli.FlagStatSuite` to `org.bdgenomics.adam.read`,
  along with the `NA12878.sam` test resource, which was otherwise unused in the
  `adam-cli` submodule.

fnothaft added a commit to fnothaft/adam that referenced this pull request Nov 15, 2016

[ADAM-1083] Cleaning up `org.bdgenomics.adam.models`.
Along with #1263 and #1264, this resolves #1083.

* Removing unused org.bdgenomics.adam.models.ReadBucket class.
* Move org.bdgenomics.adam.models.ReferencePositionPair and
  org.bdgenomics.adam.models.SingleReadBucket in to org.bdgenomics.adam.rdd.read
  and make package private.
* Clean up duplicated methods and methods that were incorrectly in companion
  singleton for SequenceDictionary and ReadGroupDictionary.
* Removed all SamReader references.
* Make writable file headers private to ADAM.
* Eliminated manual VCF parsing code in SnpTable.
* Cleaned up scaladoc for all classes and singleton objects.
* Moved `NonoverlappingRegions` test code out of `InnerBroadcastRegionJoinSuite`.
@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Nov 15, 2016

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1608/
Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/1608/
Test PASSed.

@heuermh heuermh merged commit e929676 into bigdatagenomics:master Nov 16, 2016

1 check passed

default Merged build finished.
Details
@heuermh

This comment has been minimized.

Show comment
Hide comment
@heuermh

heuermh Nov 16, 2016

Member

Thank you, @fnothaft!

Member

heuermh commented Nov 16, 2016

Thank you, @fnothaft!

fnothaft added a commit to fnothaft/adam that referenced this pull request Nov 16, 2016

[ADAM-1083] Cleaning up `org.bdgenomics.adam.models`.
Along with #1263 and #1264, this resolves #1083.

* Removing unused org.bdgenomics.adam.models.ReadBucket class.
* Move org.bdgenomics.adam.models.ReferencePositionPair and
  org.bdgenomics.adam.models.SingleReadBucket in to org.bdgenomics.adam.rdd.read
  and make package private.
* Clean up duplicated methods and methods that were incorrectly in companion
  singleton for SequenceDictionary and ReadGroupDictionary.
* Removed all SamReader references.
* Make writable file headers private to ADAM.
* Eliminated manual VCF parsing code in SnpTable.
* Cleaned up scaladoc for all classes and singleton objects.
* Moved `NonoverlappingRegions` test code out of `InnerBroadcastRegionJoinSuite`.

fnothaft added a commit to fnothaft/adam that referenced this pull request Nov 16, 2016

[ADAM-1083] Cleaning up `org.bdgenomics.adam.models`.
Along with #1263 and #1264, this resolves #1083.

* Removing unused org.bdgenomics.adam.models.ReadBucket class.
* Move org.bdgenomics.adam.models.ReferencePositionPair and
  org.bdgenomics.adam.models.SingleReadBucket in to org.bdgenomics.adam.rdd.read
  and make package private.
* Clean up duplicated methods and methods that were incorrectly in companion
  singleton for SequenceDictionary and ReadGroupDictionary.
* Removed all SamReader references.
* Make writable file headers private to ADAM.
* Eliminated manual VCF parsing code in SnpTable.
* Cleaned up scaladoc for all classes and singleton objects.
* Moved `NonoverlappingRegions` test code out of `InnerBroadcastRegionJoinSuite`.

heuermh added a commit that referenced this pull request Nov 16, 2016

[ADAM-1083] Cleaning up `org.bdgenomics.adam.models`.
Along with #1263 and #1264, this resolves #1083.

* Removing unused org.bdgenomics.adam.models.ReadBucket class.
* Move org.bdgenomics.adam.models.ReferencePositionPair and
  org.bdgenomics.adam.models.SingleReadBucket in to org.bdgenomics.adam.rdd.read
  and make package private.
* Clean up duplicated methods and methods that were incorrectly in companion
  singleton for SequenceDictionary and ReadGroupDictionary.
* Removed all SamReader references.
* Make writable file headers private to ADAM.
* Eliminated manual VCF parsing code in SnpTable.
* Cleaned up scaladoc for all classes and singleton objects.
* Moved `NonoverlappingRegions` test code out of `InnerBroadcastRegionJoinSuite`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment