-
Notifications
You must be signed in to change notification settings - Fork 13
Prepare for 2.0 release #19
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
Conversation
Oh, actually, there was the newtype iso thing that we could include here too, right? |
|
||
instance monadCostar :: Monad (Costar f a) | ||
|
||
instance distributiveCostar :: Distributive f => Distributive (Costar f a) where | ||
distribute f = Costar \g -> map ((_ $ g) <<< unCostar) f | ||
distribute f = Costar \a -> map (\(Costar g) -> g a) f |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need the Distributive
constraint on f
here?
Looks good. Yes, having something like mapNewtype :: forall new old p. (Newtype new old, Profunctor p) => p old old -> p new new would be good, but we can leave it for a minor release if you like. |
wrapIso :: forall t a p. (Newtype t a, Profunctor p) => p t t -> p a a | ||
wrapIso = dimap wrap unwrap | ||
|
||
unwrapIso :: forall t a p. (Newtype t a, Profunctor p) => p a a -> p t t |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we need a a -> t
argument here to make type inference work in some cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah good point, probably. I was going to ask though - do we need both versions here? I know you can turn a profunctor iso lens around, but you need to bring lenses in for that, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think both are potentially useful, right? Actually, are these the wrong way around?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only if you think the names should make sense... ahem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On that note, are these names any good anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think they're good.
No description provided.