instances for std Either. closes #117

[wedens]

No description provided.

[stew]

looking good. I think we also have MonadCombine when the left is a monoid (we need the monoidal zero on the left for filter)

[wedens]

I don't know how to implement MonadFilter without requiring A (left) to always have Monoid. For MonadCombine, A should always have Monoid and B should always have Semigroup. Any suggestions?

[wedens]

Or also don't implements MonadCombine btw

[wedens]

I tried to write something, but I'm not sure it's correct.

[ceedubs]

@stew (and anyone else who wants to weigh in: Is the MonadCombine that requires the left's Monoid[A] useful? It seems to me like it wouldn't be something that people would really want to use in practice.

If we do keep it, I think we can change Monad[Either[A, ?]] with MonadCombine[Either[A, ?]] to just MonadCombine[Either[A, ?]] since MonadCombine extends Monad.

[stew]

hmm, maybe not MonadCombine, but MonadFilter for using them in for comprehensions?

[xuwei-k]

First, what is the laws of MonadFilter? cats.MonadFilter is scalaz.MonadPlus?

[ceedubs]

@stew so something like this?

val errorOrInt: Either[String, Int] = Right(-2)
for {
  x <- errorOrInt if (x > 0)
} yield x + 1
// Either[String, Int] = Left("")

It just seems unlikely that this is actually what people would want. It seems like something like \/.ensure from scalaz should be used instead. I certainly can't speak for all users and what they want, but I guess it seems a bit error-prone to me.

[xuwei-k]

similar discussion in scalaz https://groups.google.com/d/topic/scalaz/NCiRRxLjanM/discussion

[wedens]

also withFilter is required to deconstruct tuples in for comprehension

[non]

@xuwei-k MonadCombine is the analogue to scalaz.MonadPlus. MonadFilter is a bit weaker.

It's a great point -- we need laws for MonadFilter! I think most of the laws revolve around how its empty method behaves.

For example: empty[A].flatMap(f) = empty[A] for any f: A => M[A].

[mpilquist]

In addition to the law @non listed, I think we also need: fa.flatMap { a => empty[B] } === empty[B] for any fa: F[A]

[non]

Right, and really my law should be generalized to empty[A].flatMap(f) = empty[B] for any f: A => M[B].

[aryairani]

@wedens Definitely wish for withFilter if it will enable deconstructing into tuples instead of needing two lines for that.

[mpilquist]

I don't think we need to solve this as part of this PR, but I think we should discuss whether we really want Either and Or to define a MonadFilter instance, or in the case of Or, a filter / withFilter. I share @ceedubs concern here. I don't think there's a relation between the Monoid[A].empty and MonadFilter[A Or ?].empty. Hence, I'd rather just define Monad for each of these types, and remove filter and withFilter.

[wedens]

Ok. I'll remove this controversial instances, so we can add them later when it'll became more clear.

[ceedubs]

Sorry we haven't added it to the contributor guide, but can you please follow the convention agreed upon in #27 and use implicit A: Order[A], B: Order[B] and then use A.compare and B.compare instead of Order[A].compare and Order[B].compare?

[wedens]

@ceedubs done. should i squash commits?

[ceedubs]

@wedens ah sorry here's one more that I missed - the Applicative.apply calls.

[ceedubs]

@wedens nah it sounds like enough cats contributors dislike changing git history that it's preferred to not squash commits once a PR is open. We need to get this into the contributor guide.

Thanks for all of the work and the prompt turnarounds. Sorry that a bunch of discussions (MonadFilter, etc) got pork-barreled into this PR discussion.

[ceedubs]

Wow that was a fast turnaround!

👍 once Travis goes green.

[wedens]

@ceedubs yeah. It's definetly should be in contributor guide as it varies from project to project. Some prefer to squash commits, others prefer not to overwrite history.

[ceedubs]

@wedens yeah you are definitely right. I'll open a PR adding this to the contributor guide right away.

Edit: #170 submitted

[non]

👍