-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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.
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
I1061 part 1 - renames in std package #1066
Changes from 1 commit
65a506c
67a846e
4b3d6f3
495bff7
9459d62
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,7 @@ package cats | |
package std | ||
|
||
trait EitherInstances extends EitherInstances1 { | ||
implicit val eitherBitraverse: Bitraverse[Either] = | ||
implicit val catsBitraverseForEither: Bitraverse[Either] = | ||
new Bitraverse[Either] { | ||
def bitraverse[G[_], A, B, C, D](fab: Either[A, B])(f: A => G[C], g: B => G[D])(implicit G: Applicative[G]): G[Either[C, D]] = | ||
fab match { | ||
|
@@ -23,7 +23,7 @@ trait EitherInstances extends EitherInstances1 { | |
} | ||
} | ||
|
||
implicit def eitherInstances[A]: Monad[Either[A, ?]] with Traverse[Either[A, ?]] = | ||
implicit def catsMonadForEither[A]: Monad[Either[A, ?]] with Traverse[Either[A, ?]] = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For this and several other instances you've somewhat arbitrarily picked the first first type class when an instanced provides several type classes. This may not be a big deal and I don't think I really have a better suggestion, but it does feel a little off. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, I used a regex replace, so it inherited the original name which didn't have the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's probably no single good solution here, but I think that including all of the type class names in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I thought about using a name like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As per #1061 (comment), perhaps the full package name should be incuded? So here, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ceedubs I don't recall any situation where this sort of thing came up. Having said that, I now wonder whether these instances are necessarily orphan, or whether we can come up with a non-orphan encoding which doesn't duplicate code all over the place. (If they were not orphan this problem wouldn't have existed in the first place.) |
||
new Monad[Either[A, ?]] with Traverse[Either[A, ?]] { | ||
def pure[B](b: B): Either[A, B] = Right(b) | ||
|
||
|
@@ -55,14 +55,14 @@ trait EitherInstances extends EitherInstances1 { | |
fa.fold(_ => lc, b => f(b, lc)) | ||
} | ||
|
||
implicit def eitherOrder[A, B](implicit A: Order[A], B: Order[B]): Order[Either[A, B]] = new Order[Either[A, B]] { | ||
implicit def catsOrderForEither[A, B](implicit A: Order[A], B: Order[B]): Order[Either[A, B]] = new Order[Either[A, B]] { | ||
def compare(x: Either[A, B], y: Either[A, B]): Int = x.fold( | ||
a => y.fold(A.compare(a, _), _ => -1), | ||
b => y.fold(_ => 1, B.compare(b, _)) | ||
) | ||
} | ||
|
||
implicit def eitherShow[A, B](implicit A: Show[A], B: Show[B]): Show[Either[A, B]] = | ||
implicit def catsShowForEither[A, B](implicit A: Show[A], B: Show[B]): Show[Either[A, B]] = | ||
new Show[Either[A, B]] { | ||
def show(f: Either[A, B]): String = f.fold( | ||
a => s"Left(${A.show(a)})", | ||
|
@@ -72,7 +72,7 @@ trait EitherInstances extends EitherInstances1 { | |
} | ||
|
||
private[std] sealed trait EitherInstances1 extends EitherInstances2 { | ||
implicit def eitherPartialOrder[A, B](implicit A: PartialOrder[A], B: PartialOrder[B]): PartialOrder[Either[A, B]] = | ||
implicit def catsPartialOrderForEither[A, B](implicit A: PartialOrder[A], B: PartialOrder[B]): PartialOrder[Either[A, B]] = | ||
new PartialOrder[Either[A, B]] { | ||
def partialCompare(x: Either[A, B], y: Either[A, B]): Double = x.fold( | ||
a => y.fold(A.partialCompare(a, _), _ => -1), | ||
|
@@ -82,7 +82,7 @@ private[std] sealed trait EitherInstances1 extends EitherInstances2 { | |
} | ||
|
||
private[std] sealed trait EitherInstances2 { | ||
implicit def eitherEq[A, B](implicit A: Eq[A], B: Eq[B]): Eq[Either[A, B]] = new Eq[Either[A, B]] { | ||
implicit def catsEqForEither[A, B](implicit A: Eq[A], B: Eq[B]): Eq[Either[A, B]] = new Eq[Either[A, B]] { | ||
def eqv(x: Either[A, B], y: Either[A, B]): Boolean = x.fold( | ||
a => y.fold(A.eqv(a, _), _ => false), | ||
b => y.fold(_ => false, B.eqv(b, _)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might have gone a little overboard here ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
catsShowForcatsForInt
=>catsShowForInt