part-2.4, 'Representable Functors' #51

Zelenya · 2018-01-28T14:54:46Z

Fixes #22

kailuowang

beautiful! Thanks!

SystemFw · 2018-01-29T14:20:21Z

There's quite a lot to review on this :)
I'll try to get to it asap.

P.S. @Zelenya Thanks a lot for all this effort! 😄

SystemFw · 2018-01-29T16:29:58Z

src/main/tut/2.4-representable-functors.md

+```
+```scala
+`C(A, X)` => `C(A, Y)`
+```


The above is pseudo haskell, so I'm not sure how to translate it. but it should probably be C[A, X]. The way you've written that's a specific type which happens to be called C(A, X) (the A and X are part of the name)

SystemFw · 2018-01-29T16:31:37Z

src/main/tut/2.4-representable-functors.md

+f ∘ h :: a -> y
+```
+```scala
+val `f ∘ h`: A => Y


This scala code is not the same as the haskell above:
in the haskell snippet a and y are universally quantified, whereas in the scala code they are concrete types called A and Y.
It should be

def `f ∘ h`[A, Y] : A => Y

SystemFw · 2018-01-29T16:33:14Z

src/main/tut/2.4-representable-functors.md

+```
+```tut:silent
+case class Reader[A, X](run: A => X)
+```


Strictly speaking the haskell code is using a type synonym, not a newtype, so the translation is a also a type synonym, not a case class. Not sure this is worth changing though

I've shamefully copy-pasted it from the 1.10 and not noticed the difference.
It was newtype Reader e a = Reader (e -> a)

right, I don't really mind too much which way you go, just wanted to point out the discrepancy :)

SystemFw · 2018-01-29T16:33:36Z

src/main/tut/2.4-representable-functors.md

+  new Functor[({type T[X] = Reader[A, X]})#T] {
+    def fmap[X, B](f: X => B)(h: Reader[A, X])
+        : Reader[A, B] =
+      Reader(x => f(h.run(x)))


Same here re: newtype

SystemFw · 2018-01-29T16:33:51Z

src/main/tut/2.4-representable-functors.md

+```
+```tut:silent
+case class Op[A, X](f: X => A)
+```


same here re: newtype

SystemFw · 2018-01-29T16:36:10Z

src/main/tut/2.4-representable-functors.md

+αx :: C(a, x) -> F x
+```
+```scala
+val αx: `C(A, X)` => F[X]


A and X are type variables in the haskell code, in yours C(A, X) is a concrete type named C(A, X) (there the X has no connection with the latter X), and X is a concrete type called X

SystemFw · 2018-01-29T16:40:53Z

src/main/tut/2.4-representable-functors.md

+```
+```scala
+def alpha[X]: (A => X) => F[X]
+```


this is incorrect. The haskell version has type

alpha :: forall a. (forall x. (a -> x) -> F x)

so it's a natural transformation between (-> a) and F. The scala code is a normal function where X is a type parameter, F is fixed (correctly), and A is fixed (incorrectly). And the returned type is not polymorphic enough (it's not first class polymorphic).

To encode this you need a type such as the one in cats:

trait FunctionK[G[_], H[_]] { def apply[A](ga: G[A]): H[A] }

and alpha becomes

def alpha[A]: FunctionK[A => ?, F]

SystemFw · 2018-01-29T16:41:48Z

src/main/tut/2.4-representable-functors.md

+```scala
+fmap(f)(alpha(h)) == alpha(f compose h)
+```
+................


The code for the two laws will probably change once you apply the change above

SystemFw · 2018-01-29T16:42:17Z

src/main/tut/2.4-representable-functors.md

+```
+```scala
+def beta[X]: F[X] => (A => X)
+```


Same comment re: natural transformation applies to beta

SystemFw · 2018-01-29T16:43:05Z

src/main/tut/2.4-representable-functors.md

+```tut:silent
+def alpha[X]: (Int => X) => List[X] =
+  h => List(12).map(h)
+```


same, this should be

def alpha: FunctionK[Int => ?, List] = ...and so on

SystemFw · 2018-01-29T16:43:36Z

src/main/tut/2.4-representable-functors.md

+```
+```scala
+def beta[X]: List[X] => (Int => X)
+```


same:

def beta: FunctionK[List, Int => ?]

SystemFw · 2018-01-29T16:45:16Z

src/main/tut/2.4-representable-functors.md

+    h: () => A,
+    t: () => Stream[A]
+    ) extends Stream[A]
+```


This is a general thing about datatypes, but in scala one would never have a trait with only one subclass, just go with

case class Stream[X](h: () => X, t: () => Stream[X])

SystemFw · 2018-01-29T16:54:13Z

src/main/tut/2.4-representable-functors.md

+```
+```tut:silent
+trait Representable[F[_]] {
+  type Rep[F[_]]


This should be lower kinded, type Rep
Type families and abstract types work differently

SystemFw · 2018-01-29T16:54:47Z

src/main/tut/2.4-representable-functors.md

+  type Rep[F[_]]
+
+  def tabulate[X](f: Rep[F] => X): F[X]
+


SystemFw · 2018-01-29T16:55:46Z

src/main/tut/2.4-representable-functors.md

+
+  def index[X]: F[X] => Rep[F] => X
+}
+```


SystemFw · 2018-01-29T16:56:18Z

src/main/tut/2.4-representable-functors.md

+        else index(bs())(n - 1)
+    }
+  }
+```


This will become:

implicit def streamRepresentable: Representable[SStream]{type Rep = Int} = new Representable[SStream] { type Rep = Int def tabulate[X](f: Rep => X) : SStream[X] = SStream[X]( () => f(0), () => tabulate(f compose (_ + 1))) def index[X] : SStream[X] => Rep => X = s => n => { if (n == 0) s.h() else index(s.t())(n - 1) } } }

SystemFw

Sorry for the spam, didn't realise I wasn't doing it all at once in a review

Zelenya · 2018-02-01T20:02:25Z

@SystemFw Ready for the round 2.

SystemFw

Just a couple of small changes left! Thanks a lot for your continued effort :)

SystemFw · 2018-02-01T22:19:30Z

src/main/tut/2.4-representable-functors.md

+αx :: C(a, x) -> F x
+```
+```scala
+def α[X]: C[A, X] => F[X]


Missing an A type parameter

SystemFw · 2018-02-01T22:21:36Z

src/main/tut/2.4-representable-functors.md

+In order to make a universal transformation,
+another type needs to be introduced.
+FunctionK (~>) transforms values from one
+first-order-kinded type into another:


first-order-kinded type is confusing. Also, the description would also apply to
def foo[F[_], G[_], A](fa: F[A]): G[A], which isn't a natural transformation: it's really the higher rank polymorphism that matters here. Personally I'd just remove the second paragraph and link at the docs for FunctionK

SystemFw · 2018-02-01T22:23:42Z

src/main/tut/2.4-representable-functors.md

+    index (Cons b bs) n = if n == 0 then b else index bs (n - 1)
+```
+```tut:silent
+implicit def streamRepresentable =


I would add the explicit type here, for two reasons:

it's good practice to always annotate implicits

it's someone decides to add a signature of Representable[Stream] the code will compile, but be broken as the refinement is lost

SystemFw · 2018-02-02T09:18:08Z

src/main/tut/2.4-representable-functors.md

+```
+```tut:silent
+implicit def streamRepresentable
+    : Representable[Stream] =


Ok I swear this is the last one 😂
The type of this is Representable[Stream]{type Rep = Int}, you want to keep the Rep = Int refinement

Zelenya · 2018-02-02T11:22:16Z

@SystemFw final round? 😄

SystemFw

🎉 🎉

Thank you so much! Sorry for the picky review

Zelenya force-pushed the part-2.4-representable-functors branch from c47f5d3 to 6ba75ec Compare January 28, 2018 15:31

kailuowang approved these changes Jan 29, 2018

View reviewed changes

SystemFw reviewed Jan 29, 2018

View reviewed changes

src/main/tut/2.4-representable-functors.md Outdated

type Rep[F[_]]

def tabulate[X](f: Rep[F] => X): F[X]

Copy link

Contributor

SystemFw Jan 29, 2018

Choose a reason for hiding this comment

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

Rep

SystemFw reviewed Jan 29, 2018

View reviewed changes

src/main/tut/2.4-representable-functors.md Outdated

def index[X]: F[X] => Rep[F] => X

}

```

Copy link

Contributor

SystemFw Jan 29, 2018

Choose a reason for hiding this comment

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

Rep

SystemFw reviewed Jan 29, 2018

View reviewed changes

SystemFw suggested changes Jan 29, 2018

View reviewed changes

Zelenya force-pushed the part-2.4-representable-functors branch 3 times, most recently from db6c332 to 98fa2e8 Compare February 1, 2018 19:44

SystemFw suggested changes Feb 1, 2018

View reviewed changes

Zelenya force-pushed the part-2.4-representable-functors branch from 98fa2e8 to 20d632c Compare February 2, 2018 07:25

SystemFw reviewed Feb 2, 2018

View reviewed changes

Zelenya force-pushed the part-2.4-representable-functors branch from 20d632c to 27d5fb3 Compare February 2, 2018 11:18

add part-2.4-representable-functors

a41bd79

Zelenya force-pushed the part-2.4-representable-functors branch from 27d5fb3 to a41bd79 Compare February 2, 2018 11:20

SystemFw approved these changes Feb 2, 2018

View reviewed changes

SystemFw merged commit 0036bf9 into typelevel:master Feb 2, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

part-2.4, 'Representable Functors' #51

part-2.4, 'Representable Functors' #51

Zelenya commented Jan 28, 2018

kailuowang left a comment

SystemFw commented Jan 29, 2018

SystemFw Jan 29, 2018 •

edited

Loading

SystemFw Jan 29, 2018 •

edited

Loading

SystemFw Jan 29, 2018

Zelenya Jan 29, 2018

SystemFw Jan 29, 2018

SystemFw Jan 29, 2018

SystemFw Jan 29, 2018

SystemFw Jan 29, 2018

SystemFw Jan 29, 2018 •

edited

Loading

SystemFw Jan 29, 2018

SystemFw Jan 29, 2018

SystemFw Jan 29, 2018 •

edited

Loading

SystemFw Jan 29, 2018 •

edited

Loading

SystemFw Jan 29, 2018 •

edited

Loading

SystemFw Jan 29, 2018 •

edited

Loading

SystemFw Jan 29, 2018

SystemFw Jan 29, 2018

SystemFw Jan 29, 2018

SystemFw left a comment

Zelenya commented Feb 1, 2018

SystemFw left a comment

SystemFw Feb 1, 2018

SystemFw Feb 1, 2018

SystemFw Feb 1, 2018

SystemFw Feb 2, 2018

Zelenya commented Feb 2, 2018 •

edited

Loading

SystemFw left a comment

part-2.4, 'Representable Functors' #51

part-2.4, 'Representable Functors' #51

Conversation

Zelenya commented Jan 28, 2018

kailuowang left a comment

Choose a reason for hiding this comment

SystemFw commented Jan 29, 2018

SystemFw Jan 29, 2018 • edited Loading

Choose a reason for hiding this comment

SystemFw Jan 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SystemFw Jan 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SystemFw Jan 29, 2018 • edited Loading

Choose a reason for hiding this comment

SystemFw Jan 29, 2018 • edited Loading

Choose a reason for hiding this comment

SystemFw Jan 29, 2018 • edited Loading

Choose a reason for hiding this comment

SystemFw Jan 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SystemFw left a comment

Choose a reason for hiding this comment

Zelenya commented Feb 1, 2018

SystemFw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zelenya commented Feb 2, 2018 • edited Loading

SystemFw left a comment

Choose a reason for hiding this comment

SystemFw Jan 29, 2018 •

edited

Loading

SystemFw Jan 29, 2018 •

edited

Loading

SystemFw Jan 29, 2018 •

edited

Loading

SystemFw Jan 29, 2018 •

edited

Loading

SystemFw Jan 29, 2018 •

edited

Loading

SystemFw Jan 29, 2018 •

edited

Loading

SystemFw Jan 29, 2018 •

edited

Loading

Zelenya commented Feb 2, 2018 •

edited

Loading