Adds an As class which represents subtyping relationships (Liskov) #1564

stew · 2017-03-18T17:02:40Z

This is a direct port of the Liskov class from scalaz. I named it As to be
similar to our new Is class representing type equality, but there are aliases
named <~< and Liskov

This is a direct port of the Liskov class from scalaz. I named it `As` to be similar to our new `Is` class representing type equality, but there are aliases named `<~<` and `Liskov`

johnynek · 2017-03-18T17:49:19Z

core/src/main/scala/cats/evidence/As.scala

+   * to a function, such as F in: `type F[-B] = B => String`. In this
+   * case, we could use A As B to turn an F[B] Into F[A].
+   */
+  def subst[F[-_]](p: F[B]): F[A]


We use substitute in Is.

johnynek · 2017-03-18T17:49:58Z

core/src/main/scala/cats/evidence/As.scala

+ *  The original contribution to scalaz came from Jason Zaugg
+ */
+sealed abstract class As[-A, +B] {
+  def apply(a: A): B = As.witness(this)(a)


I think we called this coerce in Is. Can we use the same for both?

johnynek · 2017-03-18T17:52:36Z

core/src/main/scala/cats/evidence/As.scala

+  /**
+   * Use this relationship to widen the output type of a Function1
+   */
+  def onF[X](fa: X => A): X => B = As.co2_2[Function1, B, X, A](this)(fa)


Why is this one on the instance but generally the rest are on the object?

johnynek · 2017-03-18T17:52:59Z

core/src/main/scala/cats/evidence/As.scala

+  /*
+   * Subtyping forms a category 
+   */
+  implicit val liskov: Category[<~<] = new Category[<~<] {


I don't think we have a category for Is. we should be consistent.

johnynek · 2017-03-18T17:55:40Z

core/src/main/scala/cats/evidence/As.scala

+  /**
+   * Subtyping is reflexive 
+   */
+  implicit def refl[A]: (A <~< A) =


Why do we need this and isa?

Also, in Is we use a singleton to avoid allocations.

johnynek · 2017-03-18T17:59:25Z

core/src/main/scala/cats/evidence/As.scala

+    new (A <~< A) {
+      def subst[F[-_]](p: F[A]): F[A] = p
+    }
+


We don't have a method to materialize a <:< as needed. I think we should. Also a similar unsafe method to go from <:< to As.

johnynek · 2017-03-18T18:00:13Z

core/src/main/scala/cats/evidence/As.scala

+  def co2[T[+_, _], Z, A, B](a: A <~< Z): T[A, B] <~< T[Z, B] =
+    a.subst[λ[`-α` => T[α, B] <~< T[Z, B]]](refl)
+
+  /**


If we have all these methods below I think we should have the analogous for Is.

johnynek · 2017-03-18T18:00:55Z

core/src/main/scala/cats/evidence/package.scala

@@ -2,4 +2,24 @@ package cats

 package object evidence {
  type Leibniz[A, B] = cats.evidence.Is[A, B]


Why do we use cats.evidence here but not below?

Maybe also add a === alias to parallel <~<?

johnynek · 2017-03-18T18:02:11Z

core/src/main/scala/cats/evidence/As.scala

+  /**
+   * Unsafely force a claim that A is a subtype of B 
+   */
+  def force[A, B]: A <~< B =


I don't think we should have this. You can call isa and cast if you want to do something unsafe. The cast at least shows you it is unsafe. Burying an unsafe cast in a library function seems like a bad choice to me.

remove explicit reference to package in Is type alias (for posco) add === type alias for `Is` (for sellout)

johnynek · 2017-03-19T17:56:01Z

core/src/main/scala/cats/evidence/As.scala

+   * Lift Scala's subtyping relationship
+   */
+  @inline def unsafeFromPredef[A, B >: A]: (A As B) =
+    reflAny.asInstanceOf[A As B]


I think this is actually safe (you could allocate A As B here). It is going from A <:< B that I think is correct but can't be proven via the types.

So, start with refl, then do to covariance you have A As A <: A As B so it can be returned. So I think returning refl[A] here will compile.

johnynek · 2017-03-19T18:02:51Z

tests/src/test/scala/cats/tests/AsTests.scala

+
+class AsTests extends CatsSuite {
+  import evidence._
+


Can we test resolution explicitly of a few types? Int As Any (Int, Long) As (Any, Any)

- change unsafeFromPredef to just use refl - add tests for some expected relationships such as Int <~< Any, and List[String] <~< List[Any]

stew · 2017-04-03T23:48:16Z

@johnynek I think I've covered all your requests now

sir-wabbit · 2017-04-04T01:39:42Z

I think unsafeFromPredef should be renamed to isa or reify and implemented in terms of refl. True unsafeFromPredef should have signature A <:< B => A <~< B (it's actually safe AFAIU, just uses a cast).

  implicit def reify[A, B >: A]: A <~< B = refl[A]

  def fromPredef[A, B](eq: A <:< B): A <~< B =
    unsafeForce[A, B]

johnynek · 2017-04-04T17:34:06Z

I agree with @alexknvl 's comments. I don't see how what we have currently called unsafeFromPredef is unsafe. If it is, can you comment?

Also, the coverage of this PR is pretty low. I love using this:

https://chrome.google.com/webstore/detail/codecov-extension/keefkhehidemnokodkdkejapdgfjmijf?hl=en-US

to see which methods have no tests. Very helpful (even a fun game to get everything green).

sellout · 2017-04-04T20:54:09Z

I think this is the last thing that needs to be merged to get a Monocle-Cats PR submitted.

codecov-io · 2017-04-16T18:18:06Z

Codecov Report

Merging #1564 into master will increase coverage by 0.85%.
The diff coverage is 92.3%.

@@            Coverage Diff             @@
##           master    #1564      +/-   ##
==========================================
+ Coverage   93.08%   93.93%   +0.85%     
==========================================
  Files         250      242       -8     
  Lines        3989     4122     +133     
  Branches      136      161      +25     
==========================================
+ Hits         3713     3872     +159     
+ Misses        276      250      -26

Impacted Files	Coverage Δ
core/src/main/scala/cats/evidence/As.scala	`92.3% <92.3%> (ø)`
core/src/main/scala/cats/ApplicativeError.scala	`86.66% <0%> (-13.34%)`	⬇️
core/src/main/scala/cats/syntax/cartesian.scala	`75% <0%> (-10.72%)`	⬇️
core/src/main/scala/cats/Applicative.scala	`75% <0%> (-8.34%)`	⬇️
core/src/main/scala/cats/Reducible.scala	`93.22% <0%> (-1.61%)`	⬇️
laws/src/main/scala/cats/laws/discipline/Eq.scala	`84.37% <0%> (-0.25%)`	⬇️
core/src/main/scala/cats/instances/list.scala	`100% <0%> (ø)`	⬆️
core/src/main/scala/cats/syntax/foldable.scala	`100% <0%> (ø)`	⬆️
core/src/main/scala/cats/data/StateT.scala	`97.43% <0%> (ø)`	⬆️
core/src/main/scala/cats/data/OptionT.scala	`100% <0%> (ø)`	⬆️
... and 58 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 19c3a37...3954ec7. Read the comment docs.

… derrived from what we have, and I can't find anyone using them in the wild)

sir-wabbit · 2017-04-22T00:35:06Z

May I also suggest not adding Liskov alias? I think it might be a good idea to reserve some name space in case bounded versions of Is and As are added (they could be named Leibniz and Liskov respectively).

kailuowang · 2017-05-17T02:28:09Z

@stew it looks to me that, luckly, the compilation error on scalajs on 2.10.6 can be fixed by upgrading scalajs to 0.6.16, which is the version on master. So if you merge in master, it should probably build. UPDATE: my user error, it's still broken on scalajs 0.6.16 on 2.10.6
@johnynek it looks like that the test coverage is decent now. Wanna take another look?

kailuowang · 2017-05-19T03:21:00Z

@stew I found out the cause, the type inference in toMap lin AsTests line 7 is a bit too much for scalajs on 2.10.6 to handle. A little help like the following gets it pass.

  def toMap[A, B, X](fa: List[X])(implicit ev: X <~< (A,B)): Map[A,B] = {
    type RequiredFunc = (Map[A, B], X) => Map[A, B]
    type GivenFunc = (Map[A, B], (A, B)) => Map[A, B]
    val subst: GivenFunc <~< RequiredFunc = As.contra2_3(ev) //introduced because inference failed on scalajs on 2.10.6
    fa.foldLeft(Map.empty[A,B])(subst(_ + _))
  }

Do you have time to update the PR with this change? @johnynek do you mind taking another look?

@kailuowang

Thank you very much @kailuowang for the solutiongit diff

kailuowang · 2017-05-31T20:05:24Z

tests/src/test/scala/cats/tests/AsTests.scala

+    val f2: Any => Top = As.onF(cAsA)(f)
+  }
+
+  test("we can simultaneously narrow the input and widen the ouptut of a Function1") {


looks like we are missing a test here?

kailuowang · 2017-06-06T15:18:42Z

@stew got a bit time to finish this up? If not I would be more than happy to work on it.

kailuowang · 2017-06-12T20:01:54Z

@johnynek I continued this PR with a new one #1728. cc @sellout

@kailuowang

* Adds an As class which represents subtyping relationships This is a direct port of the Liskov class from scalaz. I named it `As` to be similar to our new `Is` class representing type equality, but there are aliases named `<~<` and `Liskov` * incorporate suggestions from Oscar (Thanksgit diff --cached)git * delete-trailing-whitespace * Minor changes to evidence/packages.scala remove explicit reference to package in Is type alias (for posco) add === type alias for `Is` (for sellout) * More enhancements for posco's feedback (thanks!) - change unsafeFromPredef to just use refl - add tests for some expected relationships such as Int <~< Any, and List[String] <~< List[Any] * ome more * add more tests, contravariant tests still pending * add contra tests and get rid fo the liftF like functions (they can be derrived from what we have, and I can't find anyone using them in the wild) * don't try to link the type in the scaladoc * fix test failure on 2.10.6 + scalajs Thank you very much @kailuowang for the solutiongit diff * added helper methods for converting Function1 * added category law tests

Adds an As class which represents subtyping relationships

6a7d719

This is a direct port of the Liskov class from scalaz. I named it `As` to be similar to our new `Is` class representing type equality, but there are aliases named `<~<` and `Liskov`

stew added the in progress label Mar 18, 2017

johnynek requested changes Mar 18, 2017

View reviewed changes

stew added 3 commits March 19, 2017 01:19

incorporate suggestions from Oscar (Thanksgit diff --cached)git

974bdf5

delete-trailing-whitespace

03411c1

Minor changes to evidence/packages.scala

399dce3

remove explicit reference to package in Is type alias (for posco) add === type alias for `Is` (for sellout)

johnynek reviewed Mar 19, 2017

View reviewed changes

stew added 3 commits March 25, 2017 11:04

Merge branch 'master' of github.com:typelevel/cats

548d220

More enhancements for posco's feedback (thanks!)

fbee74b

- change unsafeFromPredef to just use refl - add tests for some expected relationships such as Int <~< Any, and List[String] <~< List[Any]

ome more

7ffa0db

Merge branch 'master' of github.com:typelevel/cats

9ccdf88

kailuowang added this to the cats-1.0.0 milestone Apr 10, 2017

stew added 2 commits April 16, 2017 10:15

Merge branch 'master' of github.com:typelevel/cats

e76268a

add more tests, contravariant tests still pending

4e59c6e

stew added 2 commits April 16, 2017 15:00

add contra tests and get rid fo the liftF like functions (they can be…

5be6037

… derrived from what we have, and I can't find anyone using them in the wild)

don't try to link the type in the scaladoc

2c9b8b0

fix test failure on 2.10.6 + scalajs

3954ec7

Thank you very much @kailuowang for the solutiongit diff

kailuowang mentioned this pull request May 25, 2017

Prepare for the 1.0.0-MF release #1702

Merged

26 tasks

kailuowang reviewed May 31, 2017

View reviewed changes

kailuowang mentioned this pull request Jun 12, 2017

Finishing up #1564 (add As a.k.a liskov) #1728

Merged

kailuowang closed this in #1728 Jun 14, 2017

kailuowang removed the in progress label Jun 14, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds an As class which represents subtyping relationships (Liskov) #1564

Adds an As class which represents subtyping relationships (Liskov) #1564

stew commented Mar 18, 2017

johnynek Mar 18, 2017

johnynek Mar 18, 2017

johnynek Mar 18, 2017

johnynek Mar 18, 2017

johnynek Mar 18, 2017

johnynek Mar 18, 2017

johnynek Mar 18, 2017

johnynek Mar 18, 2017

johnynek Mar 18, 2017

sellout Mar 18, 2017

johnynek Mar 18, 2017

johnynek Mar 19, 2017

johnynek Mar 19, 2017

johnynek Mar 19, 2017

stew commented Apr 3, 2017

sir-wabbit commented Apr 4, 2017 •

edited

Loading

johnynek commented Apr 4, 2017

sellout commented Apr 4, 2017

codecov-io commented Apr 16, 2017 •

edited

Loading

sir-wabbit commented Apr 22, 2017 •

edited

Loading

kailuowang commented May 17, 2017 •

edited

Loading

kailuowang commented May 19, 2017 •

edited

Loading

kailuowang May 31, 2017 •

edited

Loading

kailuowang commented Jun 6, 2017

kailuowang commented Jun 12, 2017

		@@ -2,4 +2,24 @@ package cats

		package object evidence {
		type Leibniz[A, B] = cats.evidence.Is[A, B]

Adds an As class which represents subtyping relationships (Liskov) #1564

Adds an As class which represents subtyping relationships (Liskov) #1564

Conversation

stew commented Mar 18, 2017

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

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

stew commented Apr 3, 2017

sir-wabbit commented Apr 4, 2017 • edited Loading

johnynek commented Apr 4, 2017

sellout commented Apr 4, 2017

codecov-io commented Apr 16, 2017 • edited Loading

Codecov Report

sir-wabbit commented Apr 22, 2017 • edited Loading

kailuowang commented May 17, 2017 • edited Loading

kailuowang commented May 19, 2017 • edited Loading

kailuowang May 31, 2017 • edited Loading

Choose a reason for hiding this comment

kailuowang commented Jun 6, 2017

kailuowang commented Jun 12, 2017

sir-wabbit commented Apr 4, 2017 •

edited

Loading

codecov-io commented Apr 16, 2017 •

edited

Loading

sir-wabbit commented Apr 22, 2017 •

edited

Loading

kailuowang commented May 17, 2017 •

edited

Loading

kailuowang commented May 19, 2017 •

edited

Loading

kailuowang May 31, 2017 •

edited

Loading