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

Typedpipe partition #987

Merged
merged 10 commits into from
Aug 1, 2014
Merged

Typedpipe partition #987

merged 10 commits into from
Aug 1, 2014

Conversation

rubanm
Copy link
Contributor

@rubanm rubanm commented Jul 31, 2014

Addresses #986

@rubanm
Copy link
Contributor Author

rubanm commented Jul 31, 2014

Looks like RichPipe has a partition method that does not do the same thing.
Should this one be renamed then?

@johnynek
Copy link
Collaborator

well, I think we should keep TypedPipe as close as possible to scala.Iterable or Seq, so I think it is fine.

It was probably a mistake in RichPipe, but oh well. Not all things are named the same between the two.

LGTM. Merge when green.

@rubanm
Copy link
Contributor Author

rubanm commented Jul 31, 2014

Makes sense.

Travis seems to be having some trouble pulling down deps :(

error sbt.ResolveException: unresolved dependency: org.scala-lang#scala-library;2.10.2: not found

I'll retry after a while I guess.

@johnynek
Copy link
Collaborator

change the http deps to https. We saw this earlier today.

On Thu, Jul 31, 2014 at 1:43 PM, Ruban Monu notifications@github.com
wrote:

Makes sense.

Travis seems to be having some trouble pulling down deps :(

error sbt.ResolveException: unresolved dependency:
org.scala-lang#scala-library;2.10.2: not found

I'll retry after a while I guess.


Reply to this email directly or view it on GitHub
#987 (comment).

Oscar Boykin :: @posco :: http://twitter.com/posco

*
* Sometimes what you really want is a groupBy in these cases.
*/
def partition(p: T => Boolean): (TypedPipe[T], TypedPipe[T]) = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may need some rules of thumb of when to use this V.S when to use Group operation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be partition when you want to split your 'dataflow' into two, groupBy when you want to create a grouping in the same dataflow?

Anything else that comes to mind? I'm still partially hung over in the fields API world so might be missing something.

(TypedPipe.groupBy does not exactly match scala Iterable.groupBy, gives you a Grouped and not a Map[K, TypedPipe[T]. If that were the case, then partition would be the same as groupBy on a boolean which gives you (TypedPipe[T], TypedPipe[T]) instead of a Map[Boolean, TypedPipe[T]].)

@johnynek
Copy link
Collaborator

johnynek commented Aug 1, 2014

merge when green.

@johnynek
Copy link
Collaborator

johnynek commented Aug 1, 2014

closes #986

rubanm added a commit that referenced this pull request Aug 1, 2014
@rubanm rubanm merged commit 1e73f84 into twitter:develop Aug 1, 2014
@rubanm rubanm deleted the typedpipe_partition branch August 1, 2014 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants