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

traverse: Apply -> Applicative #206

Merged
merged 1 commit into from
Dec 8, 2016
Merged

Conversation

KiaraGrouwstra
Copy link
Contributor

afaict this was a typo. very confusing for learners like me. :<

@safareli
Copy link
Member

safareli commented Nov 29, 2016

2. `f` must return a value of an Applicative

Yes it's a typo, thanks!

@rpominov
Copy link
Member

@safareli
Copy link
Member

@rpominov as you pointed out maybe we should change laws so it's consistent with type signature?

Or we could change the method to something like this?

traverse :: Applicative f, Traversable t => t a ~> (TypeRep f, a -> f b) -> f (t b)

Or we need to add FAQ.md 😄

@rpominov
Copy link
Member

Yea, +1 to at lest make it consistent.

@safareli
Copy link
Member

For Tuple z a user could provide function of form a -> Tuple z a (for example (a) => Tuple([], a)) and I think it would work properly. So we should change Applicative to Apply in law?
or use TypeRep f instead of passing a -> f a? @SimonRichardson @davidchambers @scott-christopher

@scott-christopher
Copy link
Contributor

As per the comment @rpominov linked to above, my original thoughts were to use the weaker constraints of Apply given that of had to be explicitly passed anyway meaning it was already described in the signature.

Now that we have the language to describe Applicative f => TypeRep f, it could make it clearer to ask for the full TypeRep now rather than just the of function.

@rpominov
Copy link
Member

Now that we have the language to describe Applicative f => TypeRep f, it could make it clearer to ask for the full TypeRep now rather than just the of function.

That a good idea for the future. Btw this is how it's done in static-land, so we should get this automatically if unification happen.

But for now we should at least fix the inconsistency: either make a change in the signature or in laws.

@scott-christopher
Copy link
Contributor

But for now we should at least fix the inconsistency: either make a change in the signature or in laws.

Sure, in that case I'm personally happy with this PR as it is.

@davidchambers
Copy link
Member

Now that we have the language to describe Applicative f => TypeRep f, it could make it clearer to ask for the full TypeRep now rather than just the of function.

I'm in favour of this change. It's beyond the scope of this pull request, though.

Copy link
Member

@safareli safareli left a comment

Choose a reason for hiding this comment

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

I think this is ready to be merged.

I have created #207 to discuss changing of a -> f a to TypeRep f.

@safareli
Copy link
Member

safareli commented Dec 8, 2016

@fantasyland/core if others think this is the way to go please approve this.

@joneshf joneshf merged commit 2d88537 into fantasyland:master Dec 8, 2016
@joneshf
Copy link
Member

joneshf commented Dec 8, 2016

Thanks so much @tycho01 and every involved!

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.

6 participants