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

Instances for tuples? #631

Closed
Centaur opened this issue Nov 12, 2015 · 9 comments
Closed

Instances for tuples? #631

Centaur opened this issue Nov 12, 2015 · 9 comments
Milestone

Comments

@Centaur
Copy link

Centaur commented Nov 12, 2015

Did not find TupleInstances in cats.std. is this intentional or is it a feature to be implemented?

@ceedubs
Copy link
Contributor

ceedubs commented Nov 12, 2015

The tuple instances for Algebra type classes (Monoid, Eq, etc) should be provided by typelevel/algebra#82 which has been merged but not yet published in a stable release.

For Cats-specific instances, this is probably just to-be-implemented. There's a question of whether we want to create instances for arbitrarily-sized tuples or just create instances for Tuple2 and leave the rest to a shapeless companion project (such as kittens).

@adelbertc
Copy link
Contributor

I had concerns about the performance of the automatic derivation, but it seems the performance hit isn't terrible, if any at all.

That being said, I had added the Tuple codegen in typelevel/algebra#82 in the name of performance, but I am now swaying towards deriving it. I feel it would be a bit strange/inconsistent to have codegen'ed Tuple instances in algebra, some hand-rolled in cats, and some delegated to kittens. Personally I would like to just have everything generically derived.. unsure how that fits with algebra goal of a small library though

/cc @non @johnynek

@travisbrown
Copy link
Contributor

For what it's worth, @adelbertc, I think I still prefer codegen for tuples and other non-user-defined types, not necessarily for the sake of performance, but for clarity of the API (and API docs), general simplicity, etc.

It's also worth noting that tuples need special-casing in generic derivation for some type classes—e.g. in circe the decoders we'd get from generic derivation for tuples-as-case-classes aren't generally the ones we want.

@johnynek
Copy link
Contributor

I'm personally pretty dependency phobic. An example for us is Kryo. This library is a dependency of Hive, Spark, Scalding, Storm and probably many others. We would like to upgrade, but there are binary incompatibility problems between releases, so this means if we upgrade in scalding, we can't mix that code with spark. Since the pain points aren't very great, we just let it sit there.

Such is the current pain of libraries on the jvm. Unfortunately, for core libraries that you expect (and hope) will have lots of adoption, it is much better that they have very few, if any, dependencies.

For my money, I'd rather use code-gen and macros, which don't introduce new dependencies than fix a new, even lower level dependency beneath algebra (which I guess would be shapeless in this example).

@ceedubs
Copy link
Contributor

ceedubs commented Nov 13, 2015

Just to clarify my earlier comment: I was never advocating adding a Shapeless dependency to Cats or Algebra. I was raising the potential of pointing people toward Kittens if they wanted derived instances. Having said that, I think I agree that providing instances in Cats/Algebra itself is better, because it's easier on users.

@ceedubs
Copy link
Contributor

ceedubs commented Nov 18, 2015

Some of the items in #620 can't really be added until we have tuple instances for these type classes. For example the Show[WriterT[F, A, B]] instances requires a Show[F[(A, B)]] instance.

I'd vote for adding at least the 2-tuple cases, and preferably codegen for the N-tuple cases.

@ceedubs ceedubs added this to the cats-1.0.0 milestone Nov 19, 2015
@lvicentesanchez
Copy link

Today was one of those days where you had to re-implement again instances for a TupleN (Tuple2 actually). What's the latest on this? Are Tuple2 instances going to be added? Should there be an issue just for adding Tuple2 instances?

@travisbrown
Copy link
Contributor

@lvicentesanchez I was just taking a look at this, and it seem like using codegen will be messy until we have a dependency on a version of Algebra that includes #82 (or we have a cats-kernel). It doesn't seem very useful to provide Monad, Show, etc. instances but not stuff like Eq, but it also doesn't seem reasonable to duplicate codegen-ed instances that are in Algebra's (unreleased) master.

My feeling is that it makes the most sense to go ahead and publish 0.4.0 without these and wait until the dust settles on the cats-kernel business.

@ceedubs
Copy link
Contributor

ceedubs commented May 6, 2016

Tuple instances should be available as of cats 0.5.0. I'm going to go ahead and close this out. Please feel free to open up a separate issue if you run into any specific instances that are missing.

@ceedubs ceedubs closed this as completed May 6, 2016
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

No branches or pull requests

6 participants