-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add some bifunctors-style newtypes #22
Conversation
/cc @safareli |
src/Data/Profunctor/Join.purs
Outdated
|
||
-- | Turns a `Profunctor` into a `Invariant` functor by equating the two type | ||
-- | arguments. | ||
newtype Join p a = Join (p a a) |
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 wonder if it's worth providing the adjoint, which constructs a profunctor from an invariant.
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.
Personally, I'd rather just remove Invariant
, but that's a separate, larger conversation 😄
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.
Wait! I just realised something, this is purescript-deprecated/purescript-monoid#34 if we provide
instance semigroupJoin :: Semigroupoid p => Semigroup (Join p a) where
append (Join a) (Join b) = Join (a <<< b)
instance monoidJoin :: Category p => Monoid (Join p a) where
mempty = Join id
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.
👍
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.
Any idea what the adjoint would look like? I just took a stab at it, but not really sure what I'm doing there and couldn't figure out how to implement dimap
.
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 found the previous discussion of this here.
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 yeah! I remember now. Will add that too then.
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.
Feel free to give it a better name 😄
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.
Cojoin
was all I had, I'm not sure it's any better 😆
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.
Split
possibly? Since we're trying to split the type argument into two.
👍 LGTM. Curious though - is there a use case for these? |
Uhh... most of them are pretty speculative 😉 It came from a discussion with @safareli, he was asking if we could impose Then when I started on that, I figured that profunctors are still a bifunctors, just not in the purescript sense, so I went back to BF to see which of the various newtypes may translate to PF. So |
Do you think the names |
there is already Joker and Clown defined in https://pursuit.purescript.org/packages/purescript-bifunctors |
I'm happy to change them, just figured since they work the same way for bifunctors, reusing the names here was reasonable. |
There we go, |
|
||
newtype Split f a b = Split (Exists (SplitF f a b)) | ||
|
||
data SplitF f a b x = SplitF (a -> x) (x -> b) (f x) |
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.
Can we hide the constructors here?
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.
Oops, good call.
👍 LGTM, thanks! |
I just thought - should this have reused the existing newtypes? Bit late now, but probably worth considering. |
I just explored that a bit...
I think next time we do breaking releases for all the core libraries we can take a look at it again though 👍 |
The name
Cowrap
is probably a bit odd, but I haven't come up with anything better yet.