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

Add loadReadsFromSamString function into ADAMContext #1434

Closed
wants to merge 11 commits into from

Conversation

@xubo245
Copy link

@xubo245 xubo245 commented Mar 10, 2017

add loadReadsFromSamString function into ADAMContext,
add some annotation for loadReadsFromSamString

link: #1425

I have finished it in adam-0.19.0 and update for Adam-0.21.1-SNAPSHOT today.

@fnothaft @heuermh

The first and second commits is not related about link: #1425. But I do not known how to remove it, sorry.
The two commits has been PR: #1433

@AmplabJenkins
Copy link

@AmplabJenkins AmplabJenkins commented Mar 10, 2017

Can one of the admins verify this patch?

@fnothaft
Copy link
Member

@fnothaft fnothaft commented Mar 10, 2017

Jenkins, test this please.

@coveralls
Copy link

@coveralls coveralls commented Mar 10, 2017

Coverage Status

Coverage increased (+0.1%) to 76.501% when pulling 63b0a6e on xubo245:ADAM_SNAPSHOT into 07c1982 on bigdatagenomics:master.

@AmplabJenkins
Copy link

@AmplabJenkins AmplabJenkins commented Mar 10, 2017

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

Build result: FAILURE

[...truncated 16 lines...] > /home/jenkins/git2/bin/git rev-parse origin/pr/1434/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a --contains 86777ab # timeout=10 > /home/jenkins/git2/bin/git rev-parse remotes/origin/pr/1434/merge^{commit} # timeout=10Checking out Revision 86777ab (origin/pr/1434/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f 86777abd59cd3f38efc86bb19fd76eb11707852cFirst time build. Skipping changelog.Triggering ADAM-prb ? 2.3.0,2.11,1.6.1,centosTriggering ADAM-prb ? 2.6.0,2.10,1.6.1,centosTriggering ADAM-prb ? 2.3.0,2.11,2.0.0,centosTriggering ADAM-prb ? 2.3.0,2.10,2.0.0,centosTriggering ADAM-prb ? 2.6.0,2.11,2.0.0,centosTriggering ADAM-prb ? 2.6.0,2.10,2.0.0,centosTriggering ADAM-prb ? 2.3.0,2.10,1.6.1,centosTriggering ADAM-prb ? 2.6.0,2.11,1.6.1,centosADAM-prb ? 2.3.0,2.11,1.6.1,centos completed with result FAILUREADAM-prb ? 2.6.0,2.10,1.6.1,centos completed with result FAILUREADAM-prb ? 2.3.0,2.11,2.0.0,centos completed with result FAILUREADAM-prb ? 2.3.0,2.10,2.0.0,centos completed with result FAILUREADAM-prb ? 2.6.0,2.11,2.0.0,centos completed with result FAILUREADAM-prb ? 2.6.0,2.10,2.0.0,centos completed with result FAILUREADAM-prb ? 2.3.0,2.10,1.6.1,centos completed with result FAILUREADAM-prb ? 2.6.0,2.11,1.6.1,centos completed with result FAILURENotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

@xubo245
Copy link
Author

@xubo245 xubo245 commented Mar 11, 2017

I can not understand the FAILed information...It is different from mine.

Where error in my code?

@fnothaft
Copy link
Member

@fnothaft fnothaft commented Mar 14, 2017

Hi @xubo245! Similar to my comment on your other PR, this is just our source formatting check failing; you can fix this by running ./scripts/format-source. Let me know if you have any problems running that. Thank you again for the contribution, and I hope to make a review pass tomorrow!

xubo245
@xubo245
Copy link
Author

@xubo245 xubo245 commented Mar 14, 2017

I just run ./scripts/format-source success and PR in this issue.

qq 20170314150337

Is it OK?

If there are any problem, please tell me in two hour, after two hour, I need to other city and stay about two days. Thanks.

@fnothaft
Copy link
Member

@fnothaft fnothaft commented Mar 14, 2017

Jenkins, test this please.

@coveralls
Copy link

@coveralls coveralls commented Mar 14, 2017

Coverage Status

Coverage increased (+0.4%) to 76.793% when pulling 3c6a2a4 on xubo245:ADAM_SNAPSHOT into 07c1982 on bigdatagenomics:master.

@AmplabJenkins
Copy link

@AmplabJenkins AmplabJenkins commented Mar 14, 2017

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

Build result: FAILURE

[...truncated 16 lines...] > /home/jenkins/git2/bin/git rev-parse origin/pr/1434/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a --contains 57029b1 # timeout=10 > /home/jenkins/git2/bin/git rev-parse remotes/origin/pr/1434/merge^{commit} # timeout=10Checking out Revision 57029b1 (origin/pr/1434/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f 57029b136cfac82a7c1ad5f457a9f33b06e88c09First time build. Skipping changelog.Triggering ADAM-prb ? 2.3.0,2.11,1.6.1,centosTriggering ADAM-prb ? 2.6.0,2.10,1.6.1,centosTriggering ADAM-prb ? 2.3.0,2.11,2.0.0,centosTriggering ADAM-prb ? 2.3.0,2.10,2.0.0,centosTriggering ADAM-prb ? 2.6.0,2.11,2.0.0,centosTriggering ADAM-prb ? 2.6.0,2.10,2.0.0,centosTriggering ADAM-prb ? 2.3.0,2.10,1.6.1,centosTriggering ADAM-prb ? 2.6.0,2.11,1.6.1,centosADAM-prb ? 2.3.0,2.11,1.6.1,centos completed with result SUCCESSADAM-prb ? 2.6.0,2.10,1.6.1,centos completed with result SUCCESSADAM-prb ? 2.3.0,2.11,2.0.0,centos completed with result SUCCESSADAM-prb ? 2.3.0,2.10,2.0.0,centos completed with result FAILUREADAM-prb ? 2.6.0,2.11,2.0.0,centos completed with result SUCCESSADAM-prb ? 2.6.0,2.10,2.0.0,centos completed with result SUCCESSADAM-prb ? 2.3.0,2.10,1.6.1,centos completed with result SUCCESSADAM-prb ? 2.6.0,2.11,1.6.1,centos completed with result SUCCESSNotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

@fnothaft
Copy link
Member

@fnothaft fnothaft commented Mar 14, 2017

Jenkins, test this please.

@coveralls
Copy link

@coveralls coveralls commented Mar 14, 2017

Coverage Status

Coverage increased (+0.4%) to 76.793% when pulling 3c6a2a4 on xubo245:ADAM_SNAPSHOT into 07c1982 on bigdatagenomics:master.

@AmplabJenkins
Copy link

@AmplabJenkins AmplabJenkins commented Mar 14, 2017

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

@xubo245
Copy link
Author

@xubo245 xubo245 commented Mar 17, 2017

The conflicts need be resolved by me? The conflicts should be change after I download the latest version

@xubo245
Copy link
Author

@xubo245 xubo245 commented Mar 17, 2017

Is it correct that the conflicts be resolved? @fnothaft

@fnothaft fnothaft added this to the 0.23.0 milestone Mar 22, 2017
@xubo245
Copy link
Author

@xubo245 xubo245 commented Mar 27, 2017

Cloud it be merge?

It is second time that show conflicts with ADAMContext.scala

@heuermh
Copy link
Member

@heuermh heuermh commented Mar 27, 2017

@xubo245 sorry about the conflicts, we'll review and help rebase/squash commits after the 0.22.0 release is cut tomorrow.

@xubo245
Copy link
Author

@xubo245 xubo245 commented Apr 9, 2017

@heuermh Clould you help me to deal with this commits ?

@xubo245
Copy link
Author

@xubo245 xubo245 commented Apr 14, 2017

This PR has more than a month...

@xubo245
Copy link
Author

@xubo245 xubo245 commented Apr 15, 2017

Cloud you help me to deal with this commits ? @fnothaft @heuermh

@heuermh
Copy link
Member

@heuermh heuermh commented Apr 17, 2017

@xubo245 I took a closer look at this last week. Even with the commits straightened out, I feel like I'm missing the use case. What would this provide that say the Pipe API does not?

@xubo245
Copy link
Author

@xubo245 xubo245 commented Apr 17, 2017

the use case: When we invoke read mapping tools in spark, read mapping return RDD(sam format), It need to transform Adam. If write to local Filesystem, it waste time...
It has been described in #1425

I do not known what is the Pipe API .

@fnothaft fnothaft modified the milestones: 0.24.0, 0.23.0 May 11, 2017
@heuermh heuermh added this to Triage in Release 0.24.0 Jan 4, 2018
@fnothaft
Copy link
Member

@fnothaft fnothaft commented Jan 9, 2018

Closing in favor of sending users to the pipe API.

@fnothaft fnothaft closed this Jan 9, 2018
@heuermh heuermh moved this from Triage to Completed in Release 0.24.0 Jan 9, 2018
@xubo245
Copy link
Author

@xubo245 xubo245 commented Jan 10, 2018

@fnothaft You require me raise this PR before, but now don't merge...

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

Successfully merging this pull request may close these issues.

None yet

5 participants