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

How to get a Map instance from Guava Funnel? #904

Closed
LeifW opened this issue Jan 26, 2024 · 3 comments · Fixed by #936
Closed

How to get a Map instance from Guava Funnel? #904

LeifW opened this issue Jan 26, 2024 · 3 comments · Fixed by #936

Comments

@LeifW
Copy link
Contributor

LeifW commented Jan 26, 2024

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
Copy link
Contributor

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
Copy link
Contributor Author

LeifW commented Mar 21, 2024

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?

@RustedBones
Copy link
Contributor

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

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 a pull request may close this issue.

2 participants