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-1745] Adding LeftOuterShuffleRegionJoinAndGroupByLeft and tests #1746

Conversation

@devin-petersohn
Copy link
Member

@devin-petersohn devin-petersohn commented Sep 26, 2017

Resolves #1745. Depends on #1741 .

@AmplabJenkins
Copy link

@AmplabJenkins AmplabJenkins commented Sep 26, 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/2396/
Test PASSed.

@devin-petersohn devin-petersohn force-pushed the devin-petersohn:issue#1745LeftOuterGroupByLeft branch from 7f12ed2 to e7272fb Sep 26, 2017
@AmplabJenkins
Copy link

@AmplabJenkins AmplabJenkins commented Sep 26, 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/2398/
Test PASSed.

@heuermh
Copy link
Member

@heuermh heuermh commented Oct 4, 2017

I squashed and merged #1741, which hopefully was the right thing to do. You'll need to rebase and squash here.

@@ -1446,6 +1446,131 @@ trait GenomicRDD[T, U <: GenomicRDD[T, U]] extends Logging {
}

/**
* Performs a sort-merge left outer join between this RDD and another RDD.

This comment has been minimized.

@fnothaft

fnothaft Oct 4, 2017
Member

Scaladoc is missing that it performs the left outer join and rolls in a groupBy.

}

/**
* Performs a sort-merge left outer join between this RDD and another RDD.

This comment has been minimized.

@fnothaft

fnothaft Oct 4, 2017
Member

Ditto.

* overlap function. Since this is a left outer join, all values in the
* right RDD that do not overlap a value from the left RDD are dropped.
* If a value from the left RDD does not overlap any values in the right
* RDD, it will be paired with a `None` in the product of the join.

This comment has been minimized.

@fnothaft

fnothaft Oct 4, 2017
Member

None -> empty iterable.

* overlap function. Since this is a left outer join, all values in the
* right RDD that do not overlap a value from the left RDD are dropped.
* If a value from the left RDD does not overlap any values in the right
* RDD, it will be paired with a `None` in the product of the join.

This comment has been minimized.

@fnothaft

fnothaft Oct 4, 2017
Member

Ditto

}

/**
* Performs a sort-merge left outer join between this RDD and another RDD.

This comment has been minimized.

@fnothaft

fnothaft Oct 4, 2017
Member

Ditto.

* overlap function. Since this is a left outer join, all values in the
* right RDD that do not overlap a value from the left RDD are dropped.
* If a value from the left RDD does not overlap any values in the right
* RDD, it will be paired with a `None` in the product of the join.

This comment has been minimized.

@fnothaft

fnothaft Oct 4, 2017
Member

Ditto.

}

/**
* Performs a sort-merge left outer join between this RDD and another RDD.

This comment has been minimized.

@fnothaft

fnothaft Oct 4, 2017
Member

Ditto.

* overlap function. Since this is a left outer join, all values in the
* right RDD that do not overlap a value from the left RDD are dropped.
* If a value from the left RDD does not overlap any values in the right
* RDD, it will be paired with a `None` in the product of the join.

This comment has been minimized.

@fnothaft

fnothaft Oct 4, 2017
Member

Ditto.

@fnothaft
Copy link
Member

@fnothaft fnothaft commented Oct 4, 2017

Couple of docs changes, but otherwise LGTM

@devin-petersohn devin-petersohn force-pushed the devin-petersohn:issue#1745LeftOuterGroupByLeft branch from e7272fb to 2f52373 Oct 4, 2017
@devin-petersohn
Copy link
Member Author

@devin-petersohn devin-petersohn commented Oct 4, 2017

Resolved docs issues and rebased.

@AmplabJenkins
Copy link

@AmplabJenkins AmplabJenkins commented Oct 4, 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/2408/
Test PASSed.

@fnothaft
Copy link
Member

@fnothaft fnothaft commented Oct 4, 2017

LGTM, @heuermh please review and merge if OK on your end.

@heuermh
heuermh approved these changes Oct 4, 2017
Copy link
Member

@heuermh heuermh left a comment

Looks great!

@heuermh heuermh merged commit 45b9fe8 into bigdatagenomics:master Oct 4, 2017
1 of 2 checks passed
1 of 2 checks passed
codacy/pr Not so good... This pull request quality could be better.
Details
default Merged build finished.
Details
@heuermh
Copy link
Member

@heuermh heuermh commented Oct 4, 2017

Thank you, @devin-petersohn!

@heuermh heuermh changed the title Issue#1745 left outer group by left [ADAM-1745] Adding LeftOuterShuffleRegionJoinAndGroupByLeft and tests Oct 4, 2017
@heuermh heuermh added this to the 0.23.0 milestone Oct 4, 2017
@heuermh heuermh added this to Completed in Release 0.23.0 Jan 4, 2018
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

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