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

filterM broken? #1054

Closed
3rdLaw opened this issue May 21, 2016 · 4 comments · Fixed by #1225
Closed

filterM broken? #1054

3rdLaw opened this issue May 21, 2016 · 4 comments · Fixed by #1225
Assignees
Milestone

Comments

@3rdLaw
Copy link
Contributor

3rdLaw commented May 21, 2016

I ran the following in sbt console after a git pull:

scala> import cats.implicits._
import cats.implicits._
scala> import cats._
import cats._
scala> List(1, 2, 3).filterM { x => List(true, false) }
res0: List[Int] = List(1, 2, 3)

Compare this to scalaz's results:

import scalaz._
import scalaz.Scalaz._
List(1, 2, 3).filterM { x => List(true, false) }
res0: List[List[Int]] = List(List(1, 2, 3), List(1, 2), List(1, 3), List(1), List(2, 3), List(2), List(3), List())
@ceedubs
Copy link
Contributor

ceedubs commented May 22, 2016

@3rdLaw thanks for bringing this up. The filterM in cats indeed isn't the filterM in Haskell or scalaz. It doesn't even have quite the same type signature: the one in cats is not specialized to List and returns an F[A] result as opposed to F[F[A]].

We should probably do something to avoid confusion. @non it looks like you originally added filterM - what are your thoughts?

@ceedubs ceedubs added this to the cats-1.0.0 milestone May 23, 2016
@3rdLaw
Copy link
Contributor Author

3rdLaw commented May 23, 2016

Thanks for the clarification @ceedubs; I clearly assumed this was the same as Haskell/Scalaz's, so some documentation would be helpful.

Does cats have an equivalent to filterM in Haskell or scalaz?

@ceedubs
Copy link
Contributor

ceedubs commented Jun 18, 2016

@non to avoid confusion, what do you think about renaming our fliterM to flatFilter to parallel the relationship between map and flatMap?

@ceedubs
Copy link
Contributor

ceedubs commented Jun 18, 2016

@non on second thought, can you think of any practical example of using the method as it currently exists in cats? Option is about the only type that I imagine myself using it with, and in that case it would be pretty easy to use flatMap with filter instead.

ceedubs added a commit to ceedubs/cats that referenced this issue Jul 15, 2016
Resolves typelevel#1054.

This simply removes `MonadFilter.filterM` to prevent confusion due to
this `filterM` being different than that from Haskell or Scalaz.

This version of `filterM` doesn't seem particularly useful to me. The
only type that comes to mind for which this might be useful is `Option`,
and it's pretty easy to come up with a more straightforward
implementation for the `Option` case. A more general solution for
`filterM` is provided by typelevel#1148, but so far there doesn't seem to be any
interest in that proposal, so I don't want it to hold up resolving
@ceedubs ceedubs self-assigned this Jul 15, 2016
@non non closed this as completed in #1202 Jul 15, 2016
@non non removed the in progress label Jul 15, 2016
ceedubs added a commit to ceedubs/cats that referenced this issue Jul 23, 2016
`TraverseFilter` is a port of Haskell's Data.Witherable, and represents
structures that support doing a combined `traverse` and `filter` as a
single operation.

`FunctorFilter` is similar but is limited to a combined `map` and
`filter` as a single operation. The main reason that I've added
`FunctorFilter` is to extract the commonality between `TraverseFilter`
and `MonadFilter` so that they don't have collisions in methods such as
`filter`.

Some benefits of these type classes:

- They provide `filterM` (actually a more general version of it -
  `filterA`), which resolves typelevel#1054.
- They provide `collect` which is a handy method that is often used on
  standard library collections but had been absent in Cats.
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 a pull request may close this issue.

3 participants