-
Notifications
You must be signed in to change notification settings - Fork 309
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-2039] Adding support for writing BED format per UCSC definition #2042
Conversation
Test FAILed. 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 > git init /home/jenkins/workspace/ADAM-prb # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > git --version # timeout=10 > git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/heads/:refs/remotes/origin/ # timeout=15 > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10 > git config --add remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ # timeout=15 > git rev-parse origin/pr/2042/merge^{commit} # timeout=10 > git branch -a -v --no-abbrev --contains e1d4f1f # timeout=10Checking out Revision e1d4f1f (origin/pr/2042/merge) > git config core.sparsecheckout # timeout=10 > git checkout -f e1d4f1f0abb3cf5b75b6996a9007e2bc152ce02aFirst time build. Skipping changelog.Triggering ADAM-prb ? 2.7.3,2.11,2.2.2,ubuntuTriggering ADAM-prb ? 2.6.2,2.11,2.2.2,ubuntuADAM-prb ? 2.7.3,2.11,2.2.2,ubuntu completed with result SUCCESSADAM-prb ? 2.6.2,2.11,2.2.2,ubuntu completed with result FAILURENotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'Test FAILed. |
Jenkins, retest this please |
Test PASSed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nits, otherwise LGTM.
def saveAsUcscBed(fileName: String, | ||
asSingleFile: Boolean = false, | ||
disableFastConcat: Boolean = false, | ||
minimumScore: Double, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If these are "interpolated to 0/1000", shouldn't 0.0/1000.0 be the default values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minimumScore
is the score which interpolates to 0. maximumScore
is the score which interpolates to 1000. Output in this convention is always integers between 0 and 1000, inclusive.
targetMin: Double, | ||
targetMax: Double): Double = { | ||
|
||
val v = constrain(value, sourceMin, sourceMax) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove the constrain
function, import scala.math.{max, min}
and val v = max(min(sourceMax, value), sourceMin)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Test PASSed. |
Thank you, @fnothaft |
Fixes #2039
@benwbooth Please let me know if this helps.