spotify/magnolify

How to get a `Map` instance from Guava Funnel?

LeifW opened this issue · 3 comments

LeifW commented

I assume it'd go through iterableFunnel, but I can't seem to make the types work (e.g. that expects the collection to have a single type param).

Just trying to get it to encode a tuple or collection of tuples seems challenging.

implicitly[Funnel[(String, String)]]

fails to find and instance with semiauto._ in scope.

With auto._ in scope,

implicitly[Funnel[List[(String, String)]]]

gets "ambiguous implicit values":

[errror]  both macro method genFunnel in package auto of type [T]com.google.common.hash.Funnel[T]
[error]  and method iterableFunnel in trait FunnelImplicits of type [T, C[_]](implicit fnl: com.google.common.hash.Funnel[T], ti: C[T] => Iterable[T]): com.google.common.hash.Funnel[C[T]]
[error]  match expected type com.google.common.hash.Funnel[List[(String, String)]]
[error]   val listFunnel: Funnel[List[(String, String)]] = implicitly[Funnel[List[(String, String)]]]
RustedBones commented

Hi @LeifW, sorry for the late answer.

implicitly[Funnel[(String, String)]]

won't work with semiauto. You should either use auto to retrieve the funnel implicitly or create it explicitly with

FunnelDerivation[(String, String)]

Concerning the implicit conflict for List[(String, String)], we indeed have a problem with implicit priorities. Will create a patch

LeifW commented

I'm used to "codec"s for thing in the stdlib (like tuples) already being included in a library like this out of the box - without having to "derive" them. E.g. circe generates codecs for Tuples 1-22 here: https://github.com/circe/circe/blob/series/0.14.x/project/Boilerplate.scala#L157
Saves time over doing that at compile time for library users, also.
Would you accept a PR for adding instances for tuples like that - perhaps using sbt-boilerplate?

  • 👍1
RustedBones commented

Ok I get it now. We handle tuple like other generic products instead of providing a default implicit.

If we add those, we should do the same in all magnolify modules. So far most of other modules are not affected by this because there is no semiauto/auto separation. IMHO we should do this split AND provide tuple implicits for the semiauto package.

I think it is worth a discussion and gathering other opinion before you get started :) cc: @spotify/flatmap

  • 👍1