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.

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

TheseT monad transformer (plus IO instances and tests) #1162

Merged
merged 15 commits into from Jun 3, 2016

Conversation

oxbowlakes
Copy link
Contributor

@oxbowlakes oxbowlakes commented May 16, 2016

TheseT is a monad transformer for \&/ (pronounced these).

  • A \&/ B represents having either an A, or a B, or both
  • the monad transformer TheseT[M, A, B] is essentially an ops class for a M[A \&/ B]. TheseT is both a monad and it supports many of the operations one finds on \&/

@oxbowlakes
Copy link
Contributor Author

Not sure what this failure is - it doesn't look obvious that it's actually anything to do with the PR (appears to be in MiMa and something to do with the JS backend)

@@ -256,6 +256,9 @@ object ScalazArbitrary {
implicit def eitherTArb[F[+_], A, B](implicit A: Arbitrary[F[A \/ B]]): Arbitrary[EitherT[F, A, B]] =
Functor[Arbitrary].map(A)(EitherT[F, A, B](_))

implicit def theseTArb[F[+_], A, B](implicit A: Arbitrary[F[A \&/ B]]): Arbitrary[TheseT[F, A, B]] =
Copy link
Member

Choose a reason for hiding this comment

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

implicit def theseTArb[F[_], A, B]

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 have made this change

@vmarquez
Copy link
Member

👍🏼

def &&&[AA >: A, C](t: TheseT[F, AA, C])(implicit M: Semigroup[AA], F: Apply[F]): TheseT[F, AA, (B, C)]
= TheseT(F.apply2(run, t.run)(_ &&& _))

def ===[AA >: A, BB >: B](x: TheseT[F, AA, BB])(implicit EA: Equal[AA], EB: Equal[BB], F: Apply[F]): F[Boolean]
Copy link
Member

Choose a reason for hiding this comment

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

I think we should define Equal[TheseT[F, A, B] instance and then remove this method. WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

I agree, otherwise 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

EitherT defines both === as an instance method on EitherT (along the lines of the above) and also a Equal instance (in EitherTInstances) [1]. This is presumably a mistake [2] and I took the view that the former was more useful because the most common value for F is presumably IO (or a stack with IO in it) and thus there will be no implicit Equal[EitherT[F, A, B]]

I do agree that the instance method === with a return type other than Boolean is a bit iffy; I was just trying to keep TheseT as close to EitherT as possible

[1] -

implicit def eitherTEqual[F[_], A, B](implicit F0: Equal[F[A \/ B]]): Equal[EitherT[F, A, B]] 
  = F0.contramap((_: EitherT[F, A, B]).run)

[2] - just because et1 === et2 is never using the typeclass but the instance method

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 did as you ask because I just can't take the silent treatment anymore

Copy link
Member

Choose a reason for hiding this comment

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

sorry for the delay. 75935e138206a8fbf is LGTM

type TheseTListInt[A] = TheseT[List, Int, A]
type TheseTOptionInt[A] = TheseT[Option, Int, A]

implicit def TheseTListIntEqual[A: Equal]: Equal[TheseTListInt[A]] = new Equal[TheseTListInt[A]] {
Copy link
Member

Choose a reason for hiding this comment

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

I think this is unnecessary because now there is implicit def theseTEqual in ThereT.scala.

@xuwei-k xuwei-k merged commit c91edfa into scalaz:series/7.2.x Jun 3, 2016
shawjef3 pushed a commit to shawjef3/scalaz that referenced this pull request Feb 20, 2017
* Added TheseT and MonadIO/LiftIO/MonadCatchIO instances

* Added tests for laws for TheseT

* Added EitherT.toThese

* requested removing variance annotation

* requested removing the TheseTFunctions trait

* Added TheseT and MonadIO/LiftIO/MonadCatchIO instances

* Added tests for laws for TheseT

* Added EitherT.toThese

* requested removing variance annotation

* requested removing the TheseTFunctions trait

* use kind projector for M[_[_], _]

* removed === in favour of Equal[TheseT[?[_], ?, ?]

* Removed unnecessary implicit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants