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

[ADAM-1083] Cleaning up `org.bdgenomics.adam.models`. #1267

Merged
merged 1 commit into from Nov 16, 2016

Conversation

@fnothaft
Copy link
Member

fnothaft commented Nov 15, 2016

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
Copy link

AmplabJenkins commented Nov 15, 2016

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

Build result: FAILURE

[...truncated 3 lines...]Building remotely on amp-jenkins-worker-05 (centos spark-test) in workspace /home/jenkins/workspace/ADAM-prbWiping out workspace first.Cloning the remote Git repositoryCloning repository https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git init /home/jenkins/workspace/ADAM-prb # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git --version # timeout=10 > /home/jenkins/git2/bin/git -c core.askpass=true fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/heads/:refs/remotes/origin/ # timeout=15 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10 > /home/jenkins/git2/bin/git config --add remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git -c core.askpass=true fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ # timeout=15 > /home/jenkins/git2/bin/git rev-parse origin/pr/1267/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a --contains 7ffa285253ccc23de7b9d9d7bf1a630c1502ddab # timeout=10 > /home/jenkins/git2/bin/git rev-parse remotes/origin/pr/1267/merge^{commit} # timeout=10Checking out Revision 7ffa285253ccc23de7b9d9d7bf1a630c1502ddab (origin/pr/1267/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f 7ffa285253ccc23de7b9d9d7bf1a630c1502ddabFirst time build. Skipping changelog.Triggering ADAM-prb ? 2.6.0,2.11,1.5.2,centosTriggering ADAM-prb ? 2.6.0,2.10,1.5.2,centosTouchstone configurations resulted in FAILURE, so aborting...Notifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

@fnothaft fnothaft force-pushed the fnothaft:issues/1083-models branch from 2a31383 to 97e7c1e Nov 15, 2016
Copy link
Member

heuermh left a comment

Minor nits. Thank you for the heroic documentation efforts, my friend. :)

*
* @param regionable The input value
* @tparam U
* @return a boolean -- the input value should only participate in the regionJoin if the return value

This comment has been minimized.

Copy link
@heuermh

heuermh Nov 15, 2016

Member

@return True if ... might read better

* corresponding information in the type of 'value', because otherwise it'd be
* difficult to extract the correct type for Byte and NumericSequence values.
*
* This class is roughly analogous to Picard's SAMTagAndValue.

This comment has been minimized.

Copy link
@heuermh

heuermh Nov 15, 2016

Member

Picard → htsjdk

*/
def reverseComplement(s: String, notFound: (Char => Symbol) = ((c: Char) => Symbol(c, c))) = {
s.map(x => Try(apply(x)).getOrElse(notFound(x)).complement).reverse
}

/** number of symbols in the alphabet */
/**
* The number of symbols in the alphabet

This comment has been minimized.

Copy link
@heuermh

heuermh Nov 15, 2016

Member

end with period

def contains(other: ReferenceRegion): Boolean = {
orientation == other.orientation &&
referenceName == other.referenceName &&
start <= other.start && end >= other.end
}

/**
* Checks is our region overlaps (wholly or partially) another region.

This comment has been minimized.

Copy link
@heuermh

heuermh Nov 15, 2016

Member

is → if

@AmplabJenkins
Copy link

AmplabJenkins commented 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/1602/
Test PASSed.

@fnothaft fnothaft force-pushed the fnothaft:issues/1083-models branch from 97e7c1e to 3fa50a9 Nov 15, 2016
@AmplabJenkins
Copy link

AmplabJenkins commented 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/1610/
Test PASSed.

@fnothaft fnothaft force-pushed the fnothaft:issues/1083-models branch from 3fa50a9 to f87e6a2 Nov 16, 2016
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 fnothaft force-pushed the fnothaft:issues/1083-models branch from f87e6a2 to b49c900 Nov 16, 2016
@fnothaft
Copy link
Member Author

fnothaft commented Nov 16, 2016

Thanks for the review @heuermh! I've addressed your comments, rebased, and pushed.

@AmplabJenkins
Copy link

AmplabJenkins commented Nov 16, 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/1612/
Test PASSed.

@heuermh heuermh merged commit c1b4b7d into bigdatagenomics:master Nov 16, 2016
1 check passed
1 check passed
default Merged build finished.
Details
@heuermh
Copy link
Member

heuermh commented Nov 16, 2016

Thank you, @fnothaft!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.