Port Clown, Joker, Product (Product2), Costar from bifunctors/profunctor #27
Conversation
Product was excluded because a version of it with a different kind signature already exists in this repo
This prevents module name clashes when pulling in bifunctors as a transitive dependency
Clown, Cowrap, Joker, and Wrap were ignored because all but Cowrap already exist in the Functor folder and Cowrap must exist in a different repo
Its Functor instance depends on Bifunctor, so it cannot be defined here
Wrap won't compile because it still uses `rmap`
src/Data/Functor/Clown.purs
Outdated
| import Data.Functor.FunctorRight (class FunctorRight) | ||
| import Data.Newtype (class Newtype) | ||
|
|
||
| -- | Make a `Functor` over the first argument of a `Bifunctor` |
There was a problem hiding this comment.
With the reference to `Bifunctor`, this doesn't seem quite appropriate anymore. Although honestly I can't say the wording makes a lot of sense to me in its original context either.
What would people think about changing the doc comment to something like this?
-- | Lift a one-parameter type constructor to a two-parameter type constructor
-- with a phantom second parameterThere was a problem hiding this comment.
How about
Makes a type that takes two type parameters (e.g.
Tuple) into aFunctorby operating on the first parameter rather than the second:-- Functor instance for normal Tuple map show (Tuple 1 2) == (Tuple 1 "2") -- Functor instance for Clowned Tuple map show (Clown (Tuple 1 2)) == Clown (Tuple "1" 2)
There was a problem hiding this comment.
I don't think that's what a Clown does, though? mapping over a Clown has no effect on the inner value, only the phantom argument. And I don't think there needs to be special attention paid to the case when Clown is applied to a two-parameter type versus a one-parameter type, does there? A Clown (Tuple 1 2.0) has type Clown (Tuple Int) Number _, and the Int part doesn't have any effect on further clownery, right? (It's been a while since I've read about clowns and jokers so I could be missing something important.)
There was a problem hiding this comment.
Oh, whoops. You're right.
There was a problem hiding this comment.
How about this then?
Lifts a type that takes one type parameter to a type that takes two type parameters where the second parameter is a phantom type.
data Box a = Box a -- these values are the same at runtime Box a Clown (Box a) :: Clown a Int Clown (Box a) :: Clown a String type TupleInt x = Tuple Int x -- these values are the same at runtime Tuple 4 "foo" :: TupleInt String Clown (Tuple 4 "foo") :: Clown (TupleInt String Boolean) Clown (Tuple 4 "foo") :: Clown (TupleInt String Number)
There was a problem hiding this comment.
I like the general direction, although none of that actually type-checks. (The first set of examples need to be Clown Box a Int, etc.; the second set needs to have the parentheses around TupleInt ... removed and TupleInt itself needs to be defined without the x parameter so that it isn't partially applied, or it needs to be a newtype or something.)
src/Data/Functor/Joker.purs
Outdated
| import Data.Functor.FunctorRight (class FunctorRight) | ||
| import Data.Newtype (class Newtype, un) | ||
|
|
||
| -- | Make a `Functor` over the second argument of a `Bifunctor` |
There was a problem hiding this comment.
Same comment as on Clown but with the proposed language being:
-- | Lift a one-parameter type constructor to a two-parameter type constructor
-- with a phantom first parameter
src/Data/Functor/Product2.purs
Outdated
| import Prelude | ||
| import Data.Functor.FunctorRight (class FunctorRight, rmap) | ||
|
|
||
| -- | The Product of two `Bifunctor`s. |
There was a problem hiding this comment.
Another reference to `Bifunctor` which should probably change to two-parameter type constructors or something?
src/Data/Functor/Wrap.purs
Outdated
| import Data.Functor.FunctorRight (class FunctorRight, rmap) | ||
| import Data.Newtype (class Newtype) | ||
|
|
||
| -- | Provides a `Functor` over the second argument of a `Bifunctor`. |
There was a problem hiding this comment.
over the second argument of a `FunctorRight` ?
| import Data.Functor.FunctorRight (class FunctorRight) | ||
| import Data.Newtype (class Newtype) | ||
|
|
||
| -- | `Costar` turns a `Functor` into a `Profunctor` "backwards". |
There was a problem hiding this comment.
This could mention that it also turns a Contravariant into a Bifunctor... but maybe that doesn't need to be said explicitly? I'm more on the fence with this one than with my other doc suggestions.
|
Should we specialize the second parameters of Clown, Joker, Product2, Star and Wrap to Type because of their FunctorRight instance? If yes we should then also specialize the first parameter of Costar because of the Comonad constraint in its Category instance and the first parameter of Star because of the Monad constraint in its Category instance. |
Probably. I've pushed the changes. |
|
If you want to improve the docs for |
|
Regarding these data types' kinds: I think it's a bit weird to specialise only the second parameter to |
Quoting Harry: "The real function of Wrap here is to provide a Functor instance for a data type with 2 type parameters given only a FunctorRight instance. But I can't think of a good reason why someone would provide only FunctorRight and not Functor, so I'm actually wondering if now isn't a good time to just drop Wrap entirely?" If we drop it now and need to add it later for some reason, the addition would not be a breaking change. If we don't remove it now and later find we should have removed it, that will be a breaking change.
|
I've removed |
Done. |
I read through that yesterday. I understood the idea of refactoring two mutually recursive functions that would otherwise blow the stack into versions that would no longer blow the stack. I got lost in the parts beyond that due to all the symbols. Plus, I wasn't trying my hardest to read it either. Perhaps the docs on
Those that want to understand them will. Those who get scared off by the paper will find a less advanced solution to their problem (or curiosity). |
@kl0tl I also pushed these changes in this PR. The |
| show (Product2 x y) = "(Product2 " <> show x <> " " <> show y <> ")" | ||
|
|
||
| instance functorRight :: (FunctorRight f, FunctorRight g) => FunctorRight (Product2 f g) where | ||
| rmap f (Product2 x y) = Product2 (rmap f x) (rmap f y) |
There was a problem hiding this comment.
If we're dropping Wrap, Product2 ought to have a Functor instance as well, right?
There was a problem hiding this comment.
Ah... Yeah. I'll add that.
|
I've updated the docs for Clown and Joker. When I generated the docs locally, this documentation was not showing up. I'm not sure if that's because of the link or not. We can fix it later if needed. I'd like to get this in. |
JordanMartinez
changed the title
|
This PR actually introduced an issue. Since it's been so long since I worked on this initially, I had forgotten a few things. Most importantly, |
|
First PR fixing purescript/purescript-profunctor#23
I found that
Flipcouldn't be ported here because itsFunctorinstance useslmapwhich will be defined in theBifunctorclass.I also named
Bifunctor.ProductFunctor.ProductBifor a short period of time before I realized it should be calledProduct2now. Once I realized that, I renamed it toProduct2.