Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Added merge (product) to Arrow for arrows composition #2063

Merged
merged 19 commits into from
Dec 8, 2017
Merged

Added merge (product) to Arrow for arrows composition #2063

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
28fe55a
methods merge and -< work fine, but test no!
marcobattaglia Dec 5, 2017
c2e451f
fix tests
kailuowang Dec 5, 2017
29c878c
ArrowLaws without parens around A in IsEq[F[A, (B, C)]]
Dec 5, 2017
4d88b6c
Merge pull request #1 from kailuowang/marco
marcobattaglia Dec 5, 2017
fa6aa46
added law and test for Arrow.combineAndByPass
Dec 5, 2017
4f1c522
Added two method to compose Arrows 'merge' and '-<'. See comment in …
Dec 5, 2017
d7d0d47
Merge branch 'master' of https://github.com/marcobattaglia/cats
Dec 5, 2017
5f1e1d6
Merge branch 'master' of https://github.com/marcobattaglia/cats
Dec 5, 2017
f80bcb5
Merge branch 'master' of https://github.com/marcobattaglia/cats
Dec 5, 2017
87ff6a6
added exclude [ReverseMissingMethodProblem]s for Arrow.merge, Arrow.c…
Dec 5, 2017
db2d224
solved doctest errors
marcobattaglia Dec 5, 2017
2c0bc19
Better format in scala doc html presentation. Schema of merge functio…
Dec 6, 2017
817831a
combineAndByPass has been renamed to combineAndBypass
Dec 6, 2017
91dfe26
Arrow.combineAndBypass now calculates f only one time instead twice
marcobattaglia Dec 7, 2017
17c6a9e
Arrow.combineAndBypass now calculates f only one time instead twice
marcobattaglia Dec 7, 2017
c53e368
Merge branch 'master' of https://github.com/marcobattaglia/cats
marcobattaglia Dec 7, 2017
ef5996f
deleted one line of comment
marcobattaglia Dec 7, 2017
5bd2889
the combineAndBypass '-<' function has been deleted. the merge functi…
marcobattaglia Dec 7, 2017
4caa33e
space margin of comment reformatted in Arrow as standard
marcobattaglia Dec 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions build.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,12 @@ def mimaSettings(moduleName: String) = Seq(
import com.typesafe.tools.mima.core.ProblemFilters._
Seq(
exclude[ReversedMissingMethodProblem]("cats.syntax.FoldableSyntax.catsSyntaxFoldOps"),
exclude[ReversedMissingMethodProblem]("cats.arrow.Arrow.merge"),
exclude[ReversedMissingMethodProblem]("cats.arrow.Arrow.combineAndBypass"),
exclude[ReversedMissingMethodProblem]("cats.arrow.Arrow#Ops.merge"),
exclude[ReversedMissingMethodProblem]("cats.arrow.Arrow#Ops.combineAndBypass"),
exclude[ReversedMissingMethodProblem]("cats.arrow.Arrow#Ops.&&&"),
exclude[ReversedMissingMethodProblem]("cats.arrow.Arrow#Ops.-<"),
exclude[ReversedMissingMethodProblem]("cats.Traverse.unorderedTraverse"),
exclude[ReversedMissingMethodProblem]("cats.Traverse.unorderedSequence"),
exclude[InheritedNewAbstractMethodProblem]("cats.UnorderedTraverse.unorderedTraverse"),
Expand Down
46 changes: 46 additions & 0 deletions core/src/main/scala/cats/arrow/Arrow.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,50 @@ import simulacrum.typeclass
@simulacrum.op("***", alias = true)
def split[A, B, C, D](f: F[A, B], g: F[C, D]): F[(A, C), (B, D)] =
andThen(first(f), second(g))

/**
* Create a new computation `F` that merge outputs of `f` and `g` both having the same input
*
* Example:
* {{{
* scala> import cats.implicits._
* scala> val addEmpty: Int => Int = _ + 0
* scala> val multiplyEmpty: Int => Double= _ * 1d
* scala> val f: Int => (Int, Double) = addEmpty &&& multiplyEmpty
* scala> f(1)
* res0: (Int, Double) = (1,1.0)
* }}}
*
* Note that the arrow laws do not guarantee the non-interference between the _effects_ of
* `f` and `g` in the context of F. This means that `f &&& g` may not be equivalent to `g &&& f`.
*/
@simulacrum.op("&&&", alias = true)
def merge[A, B, C](f: F[A, B], g: F[A, C]): F[A, (B, C)] = {
andThen(lift((x: A) => (x, x)), split(f, g))
}

/**
* Create a new computation `F` that apply f andThen biforks the result.
* On one way it is applied to g and on the other it is passed through.
* The final result is a tuple
* <br/><pre>
* /--out1 = f(input) ----\
* input -->{ }--->(out1,out2)
* \--out2 = g(f(input))--/
* </pre>
* Example:
* {{{
* scala> import cats.implicits._
* scala> val twoTimes: Int => Double = _ * 2d
* scala> val fiveTimes: Double => Double= _ * 5
* scala> val f: Int => (Double, Double) = twoTimes -< fiveTimes
* scala> f(2)
* res0: (Double, Double) = (4.0,20.0)
* }}}
*
*/
@simulacrum.op("-<", alias = true)
def combineAndBypass[A, B, C](f: F[A, B], g: F[B, C]): F[A, (B, C)] = {
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be andThen(f, merge (id, g)) right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. Do you prefer to eliminate the combineAndBypass or to leave it as a simple shortcut? For me it's the same, I think that it depends on how cats is generally organized. I will commit following your opinion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've tries to rewrite andThe(f, merge(d,g)) as something like andThen(f, catsSemigroupalForArrow.product(id,g)) and I've noticed that catscatsSemigroupalForArrow.product is a little bit different than merge infact the product change also the types of the resulting function.
For example the type of
catsSemigroupalForArrow.product( ((x:Int) => x * 2d), ((x:Double) => x * 5) ) is Double with Int => (Double, Double) , instead the type of `((x:Int) => x * 2d) merge ((x:Int) => x * 5)' is Int => (Double, Int). My question is: 'product' means also an operation on types, in the sense that it also work on the domain types of the function ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Your catsSemigroupalForArrow.product( ((x:Int) => x * 2d), ((x:Double) => x * 5) ) is supprising. I didn't expect that to compile. We can discuss that #2078

If you don't mind I'd prefer removing combineAndBypass. It's not providing much, typing f >>> (id &&& g) isn't too much worse than f -< g which arguably is more obscure to read. And it's a lot harder to remove a method than adding one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. I will prepare the commit to delete it. Thx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this capanion object: object Arrow { implicit def catsSemigroupalForArrow[F[_, _], A](implicit F: Arrow[F]): Semigroupal[F[A, ?]] = new Apply[F[A, ?]] { def map[B, C](fb: F[A, B])(f: B => C): F[A, C] = F.rmap(fb)(f) def ap[B, C](ff: F[A, B => C])(fb: F[A, B]): F[A, C] = F.rmap(F.andThen(F.lift((x: A) => (x, x)), F.split(ff, fb)))(tup => tup._1(tup._2)) } compiles.

andThen(lift((x: A) => (x, x)), split(f, andThen(f, g)))
Copy link
Contributor

Choose a reason for hiding this comment

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

One downside of this is that it calculates f(x) twice. I think that for pretty much any arrow you would want to override this to reuse the result of f(x).

Copy link
Contributor

@kailuowang kailuowang Dec 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I agree. I think I incline to not adding this one, and the default impl is just an alias to merge(f, f >>> g)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes you are right. It is also the same of this one:
andThen(andThen( f, lift((x: B) => (x, x)) ), split(id, g))
This last one evicts the double calculation.
Essentially: input=> f(input) => out1=> (out1,out1)=> (out1,g(out1))=>(out1,out2)
Please consider if it's better to eliminate combineAndBypass because it is possible to use merge(f, f >>> g) or if it is good to have combineAndBypass as a sort of shortcut.
I'll prepare the commit following your decision.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mmm.... I think that also merge(f, f >>> g) evaluates f twice, instead this one andThen(andThen( f, lift((x: B) => (x, x)) ), split(id, g)) evaluates f one time, so that it is not properly a simple shortcut althought the result is the same.

}
}
6 changes: 6 additions & 0 deletions laws/src/main/scala/cats/laws/ArrowLaws.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ trait ArrowLaws[F[_, _]] extends CategoryLaws[F] with StrongLaws[F] {
def splitConsistentWithAndThen[A, B, C, D](f: F[A, B], g: F[C, D]): IsEq[F[(A, C), (B, D)]] =
F.split(f, g) <-> (f.first andThen g.second)

def mergeConsistentWithAndThen[A, B, C](f: F[A, B], g: F[A, C]): IsEq[F[A, (B, C)]] =
F.merge(f, g) <-> ((F.lift((x: A) => (x, x))) andThen F.split(f, g))

def combineAndBypassConsistentWithAndThen[A, B, C](f: F[A, B], g: F[B, C]): IsEq[F[A, (B, C)]] =
F.combineAndBypass(f, g) <-> (F.lift((x: A) => (x, x)) andThen F.split(f, (f andThen g)))

private def fst[A, B](p: (A, B)): A = p._1

private def assoc[A, B, C](p: ((A, B), C)): (A, (B, C)) = (p._1._1, (p._1._2, p._2))
Expand Down
6 changes: 5 additions & 1 deletion laws/src/main/scala/cats/laws/discipline/ArrowTests.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ trait ArrowTests[F[_, _]] extends CategoryTests[F] with StrongTests[F] {
def arrow[A: Arbitrary, B: Arbitrary, C: Arbitrary, D: Arbitrary, E: Arbitrary, G: Arbitrary](implicit
ArbFAB: Arbitrary[F[A, B]],
ArbFBC: Arbitrary[F[B, C]],
ArbFAC: Arbitrary[F[A, C]],
ArbFCD: Arbitrary[F[C, D]],
ArbFDE: Arbitrary[F[D, E]],
ArbFEG: Arbitrary[F[E, G]],
Expand All @@ -31,6 +32,7 @@ trait ArrowTests[F[_, _]] extends CategoryTests[F] with StrongTests[F] {
EqFADCD: Eq[F[(A, D), (C, D)]],
EqFADCG: Eq[F[(A, D), (C, G)]],
EqFAEDE: Eq[F[(A, E), (D, E)]],
EqFABC: Eq[F[A, (B, C)]],
EqFEAED: Eq[F[(E, A), (E, D)]],
EqFACDBCD: Eq[F[((A, C), D), (B, (C, D))]]
): RuleSet =
Expand All @@ -49,7 +51,9 @@ trait ArrowTests[F[_, _]] extends CategoryTests[F] with StrongTests[F] {
"arrow exchange" -> forAll(laws.arrowExchange[A, B, C, D] _),
"arrow unit" -> forAll(laws.arrowUnit[A, B, C] _),
"arrow association" -> forAll(laws.arrowAssociation[A, B, C, D] _),
"split consistent with andThen" -> forAll(laws.splitConsistentWithAndThen[A, B, C, D] _)
"split consistent with andThen" -> forAll(laws.splitConsistentWithAndThen[A, B, C, D] _),
"merge consistent with andThen" -> forAll(laws.mergeConsistentWithAndThen[A, B, C] _),
"combineAndBypass consistent with andThen" -> forAll(laws.combineAndBypassConsistentWithAndThen[A, B, C] _)
)
}
}
Expand Down
2 changes: 2 additions & 0 deletions laws/src/main/scala/cats/laws/discipline/CommutativeArrowTests.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ trait CommutativeArrowTests[F[_, _]] extends ArrowTests[F] {

def commutativeArrow[A: Arbitrary, B: Arbitrary, C: Arbitrary, D: Arbitrary, E: Arbitrary, G: Arbitrary](implicit
ArbFAB: Arbitrary[F[A, B]],
ArbFAC: Arbitrary[F[A, C]],
ArbFBC: Arbitrary[F[B, C]],
ArbFCD: Arbitrary[F[C, D]],
ArbFDE: Arbitrary[F[D, E]],
Expand All @@ -26,6 +27,7 @@ trait CommutativeArrowTests[F[_, _]] extends ArrowTests[F] {
EqFAD: Eq[F[A, D]],
EqFAG: Eq[F[A, G]],
EqFACB: Eq[F[(A, C), B]],
EqFABC: Eq[F[A, (B, C)]],
EqFACBC: Eq[F[(A, C), (B, C)]],
EqFACBD: Eq[F[(A, C), (B, D)]],
EqFADCD: Eq[F[(A, D), (C, D)]],
Expand Down