[ADAM-410] Alphabet model #653
Conversation
|
Can one of the admins verify this patch? |
|
Jenkins, add to whitelist and test this please. |
|
Test PASSed. |
| * if false, this alphabet will treat upper or lower case chars as the same for its symbols | ||
| * | ||
| */ | ||
| val caseSensitive: Boolean |
|
+1, this is good code. Thanks for the contribution @bryanjj! I left a few comments inline, but I think we can merge this soon. As a small nit, can you squash the commits down into a single commit, and prefix that commit with |
|
Thanks for the comments! I will make the changes and see how it looks |
|
Test FAILed. Build result: FAILUREGitHub pull request #653 of commit e99cf7e automatically merged.Notifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'[EnvInject] - Loading node environment variables.Building remotely on amp-jenkins-worker-05 (centos) in workspace /home/jenkins/workspace/ADAM-prb > git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # 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/pull/:refs/remotes/origin/pr/ > git rev-parse origin/pr/653/merge^{commit} # timeout=10 > git branch -a --contains 4e16fab7c8b7a4cded7d6adbd83792df38f9b06b # timeout=10 > git rev-parse remotes/origin/pr/653/merge^{commit} # timeout=10Checking out Revision 4e16fab7c8b7a4cded7d6adbd83792df38f9b06b (origin/pr/653/merge) > git config core.sparsecheckout # timeout=10 > git checkout -f 4e16fab7c8b7a4cded7d6adbd83792df38f9b06bFirst time build. Skipping changelog.Triggering ADAM-prb ? 1.0.4,centosTriggering ADAM-prb ? 2.2.0,centosTriggering ADAM-prb ? 2.3.0,centosADAM-prb ? 1.0.4,centos completed with result FAILUREADAM-prb ? 2.2.0,centos completed with result FAILUREADAM-prb ? 2.3.0,centos completed with result FAILURENotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'Test FAILed. |
…cs#410 removed the SequenceUtils class and updated all references. ran mvn verify and all tests pass. ran ./scripts/format-source make changes as suggested in the pull request: 1. unknown characters should throw illegalArgumentException instead of keyNotFound 2. case-insensitive changed to mean that it will map both lower and upper case characters to the same symbol instead of mapping lower to lower and upper to upper clean up import squashed commits
bryanjj
force-pushed
the
alphabet-issue-410
branch
from
c914832
to
dda0a17
Compare
|
Test PASSed. |
|
Test FAILed. Build result: FAILUREGitHub pull request #653 of commit dda0a17 automatically merged.Notifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'[EnvInject] - Loading node environment variables.Building remotely on amp-jenkins-worker-05 (centos) in workspace /home/jenkins/workspace/ADAM-prb > git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # 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/pull/:refs/remotes/origin/pr/ > git rev-parse origin/pr/653/merge^{commit} # timeout=10 > git branch -a --contains 793320a # timeout=10 > git rev-parse remotes/origin/pr/653/merge^{commit} # timeout=10Checking out Revision 793320a (origin/pr/653/merge) > git config core.sparsecheckout # timeout=10 > git checkout -f 793320a9b6763b0ba6a5d63f842f7da0f6ba88cdFirst time build. Skipping changelog.Triggering ADAM-prb ? 1.0.4,centosTriggering ADAM-prb ? 2.2.0,centosTriggering ADAM-prb ? 2.3.0,centosADAM-prb ? 1.0.4,centos completed with result SUCCESSADAM-prb ? 2.2.0,centos completed with result SUCCESSADAM-prb ? 2.3.0,centos completed with result FAILURENotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'Test FAILed. |
|
I addressed the comments. Please let me know what you think. After the squash, the build succeeded for hadoop 1.0 and 2.2 but failed for 2.3 I'm not sure if it's related to my change or something random. |
|
Jenkins, retest this please. |
|
Test PASSed. |
I was interested in contributing to this project and learning scala.
I am attempting to partially address #410
I created a new alphabet class which operates on symbols. I based the functionality on the existing SequenceUtils class which supports the following functionality:
My main concerns with this pull request are
this is my first github contribution ever so let me know what you think