ekmett / adjunctions

Simple adjunctions
http://hackage.haskell.org/package/adjunctions
Other
44 stars 27 forks source link

fmap @(Co _) should be fmapRep, not stock derived #63

Closed Icelandjack closed 4 years ago

Icelandjack commented 4 years ago

Hi (not using the bleeding edge), let's say I have

data Pair a = a :# a

instance Distributive Pair where
 distribute :: Functor f => f (Pair a) -> Pair (f a)
 distribute = distributeRep

instance Representable Pair where
 type Rep Pair = Bool

 index :: Pair a -> (Bool->a)
 index (f:#_) False = f
 index (_:#t) True  = t

 tabulate :: (Bool->a) -> Pair a
 tabulate make = make False :# make True

Let's say I want to derive some instances via Co Pair

data Pair a ..
 deriving (Functor, Applicative, Monad, MonadReader Bool)
 via Co Pair

Spot the mistake?

Because the Functor (Co f) instance is stock derived it's the only instance that doesn't make use of Representable and depends on f being a functor already

-- | Data.Functor.Rep
newtype Co f a = Co { unCo :: f a } deriving Functor

This doesn't trigger an error? Maybe I'm doing it wrong. In any case this means that we trigger a cyclic definition fmap = fmap.

I propose the following definition, which is in line with all other instances

instance Representable f => Functor (Co f) where
 fmap :: (a->a') -> (Co f a->Co f a')
 fmap = fmapRep
RyanGlScott commented 4 years ago

I agree that giving Co a Functor instance that doesn't use fmapRep is strange and inconsistent with its other class instances. I'd happily accept a pull request which changed this state of affairs.

Icelandjack commented 4 years ago

There is the pull request.

From :info we already see that Functor is an odd one out from its non Representable context. It still wouldn't hurt to show show which strategy if any an instance was derived from :classify,

>> :classify Co
stock:
 instance Functor f => Functor (Co f)

standalone:
 instance Representable f => Applicative (Co f)