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 blogpost: Tribulations of CanBuildFrom #651

Merged
merged 5 commits into from
May 30, 2017

Conversation

julienrf
Copy link
Contributor

The plan is to publish the blogpost next Monday.

@julienrf
Copy link
Contributor Author

This is how the diagram looks like locally:

image

@milessabin
Copy link
Contributor

Perhaps I'm being picky, but why use "Kind polymorphism" as the heading of a section which doesn't further discuss kind polymorphism (either as implemented in Typelevel Scala or elsewhere) and which illustrates without comment exactly the sort of arity-specific boilerplate which kind polymorphism in some form could eliminate?

@julienrf
Copy link
Contributor Author

julienrf commented May 24, 2017

@milessabin Actually I think “Kind polymorphism” is probably not an appropriate heading.

Also, unfortunately, in practice the way kind-polymorphism is implemented in Typelevel Scala does not really make things simpler:

trait MapDef[A, B, CC <: AnyKind] {
  type In
  type Out
  def map(in: In)(f: A => B): Out
}

trait IterableOps[A, CC <: AnyKind] {
  def map[B](f: A => B)(implicit mapDef: MapDef[A, B, CC] { type In = this.type }): mapDef.Out = mapDef.map(this)(f)
}

trait Map[K, V] { … }

object Map {
  implicit def mapDef[K, V, L, W]: MapDef[(K, V), (L, W), Map] { type In = Map[K, V]; type Out = Map[L, W] } = …
}

@milessabin
Copy link
Contributor

@julienrf I think changing the heading is the best bet.

Yes, the minimal kind polymorphism in TLS isn't up to this job, but I think it's worth exploring what can be done with something quite that minimal, and I think it would be worth exploring more sophisticated implementations.


The drawback of this solution is that the type signature of the `map` method looks cryptic.

In the new design we solve this problem by defining two overloads of the `map`
Copy link
Contributor

Choose a reason for hiding this comment

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

You may want to point out that this new design required improvements to type inference which are only available in 2.12.

get a `Seq[B]`.

Thus, we got rid of the cryptic method signatures while still supporting the feature
of returning a different type of result according to the type of the transformation function.
Copy link
Contributor

Choose a reason for hiding this comment

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

but we lose the ability to abstract over arbitrary map implementations in a unified way

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 this point is not very important from the point of view of users. That’s more related to our internal implementation.

@szeiger
Copy link
Contributor

szeiger commented May 26, 2017

You may also want to mention breakOut as a separate usecase.

@julienrf
Copy link
Contributor Author

About breakOut: this is an escape hatch in case the implicit resolution of CBF does not give the expected type. So, it’s not really a “feature” but a workaround related to the way CBF works.

@szeiger
Copy link
Contributor

szeiger commented May 29, 2017

I disagree. breakOut is a feature, not a workaround. It allows you to build whatever target collection type you need without copying from an intermediate collection. Can we replace all current use cases by Views? I checked for some obvious cases (like groupBy) where you would return some type that wraps That (instead of returning That directly) but they don't support CanBuildFrom. Are there others that do? These would be the cases that can't work with Views.

@julienrf
Copy link
Contributor Author

OK, I added a section about breakOut. WDYT?

~~~

`breakOut` selects a `CanBuildFrom` instance irrespective of the initial collection type.
In our case the `Map[Int, Int]` type annotation fixes the target type of the builder
Copy link
Contributor

Choose a reason for hiding this comment

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

How about "This requires the target type to be known, in this case via an explicit type ascription"?

Copy link
Member

@heathermiller heathermiller left a comment

Choose a reason for hiding this comment

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

LGTM :) Merci @julienrf and thank you @milessabin and @szeiger for your reviews! :)

@heathermiller heathermiller merged commit d59a8b2 into master May 30, 2017
@SethTisue SethTisue deleted the tribulations-canbuildfrom branch May 30, 2017 13:47
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