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

Move the Cartesian parameter for mapN et al? #1805

Closed
davegurnell opened this issue Aug 9, 2017 · 4 comments
Closed

Move the Cartesian parameter for mapN et al? #1805

davegurnell opened this issue Aug 9, 2017 · 4 comments

Comments

@davegurnell
Copy link

A proposal for the mapN syntax.

The Cartesian parameter for the mapN syntax and its cohorts is injected in the implicit conversion to TupleNCartesianOps:

https://github.com/typelevel/cats/blob/master/project/Boilerplate.scala#L265

This means that, if the compiler can't find a Cartesian for your type, it gives the error "the method mapN doesn't exist". I ran into this when updating some code using mapN on Validated because I didn't have -Ypartial-unification enabled.

If the implicit parameter was injected in the parameters to mapN instead, the error would change to "can't find a Cartesian for your type". I think is clearer and more indicactive of what's going wrong.

Does this seem like a good tweak or would it break things? Sorry if this ground has been covered already.

@edmundnoble
Copy link
Contributor

This is how most of the syntax in cats works; the more that works this way, the better IMO (#1669)

@kailuowang kailuowang added this to the 1.0.0-RC1 milestone Aug 9, 2017
@kailuowang
Copy link
Contributor

looks like the consensus is to make the change. anyone interested in drafting a PR?

@davegurnell
Copy link
Author

davegurnell commented Aug 10, 2017 via email

@DavidGregory084
Copy link
Member

I'll have a look at this since it's my fault ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants