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

Unexpected behavior of NonEmptyMap #3117

Open
vilu opened this issue Oct 17, 2019 · 2 comments
Open

Unexpected behavior of NonEmptyMap #3117

vilu opened this issue Oct 17, 2019 · 2 comments

Comments

@vilu
Copy link

vilu commented Oct 17, 2019

I noticed that NEM seems to prefer .find on Foldable rather on the concrete type when cats.implicits._ or more concretely import cats.instances.int._ is in scope. This confused me since I had everything working, I pulled in cats.implicits._ for some other reason and this broke the compile. What is the expected order of preference here?

object TreeWithCatsImplicit extends App {
  import cats.data.NonEmptyMap
  import cats.implicits._ // import cats.instances.int._
  val nem: NonEmptyMap[Int, String] = NonEmptyMap.of(
    1 -> "one",
    2 -> "two"
  )

  {
    import scala.reflect.runtime.universe.{reify, show}
    println(show(reify { nem.find(_ == "one") }.tree))
  }
  // implicits.toFoldableOps(Test.this.nem)(NonEmptyMapImpl.catsDataInstancesForNonEmptyMap(implicits.catsKernelStdOrderForInt)).find(((x$1) => x$1.$eq$eq("one")))
}

object WithCatsImplicit extends App {
  import cats.data.NonEmptyMap
  import cats.implicits._
  val nem: NonEmptyMap[Int, String] = NonEmptyMap.of(
    1 -> "one",
    2 -> "two"
  )

  println(nem.find(_ == "one"))
  // Some(one)
}

object WithoutCatsImplicit extends App {
  import cats.data.NonEmptyMap
  import cats.kernel.Order

  implicit val order: Order[Int] = Order.fromOrdering

  val nem: NonEmptyMap[Int, String] = NonEmptyMap.of(
    1 -> "one",
    2 -> "two"
  )

  println(nem.find(_ == "one"))
  //Some((1,one))
}

scala version 2.13
cats version 2.0.0

@GodPlaysChess
Copy link

GodPlaysChess commented Oct 17, 2019

should not foldable of NonEmptyMap fold over tuple (K, V) (like Seq[(K, V)])? It would fix this issue. Though I have no idea if it is even possible to write it in such way. May be through getting the evidence that A <:< (K, V) or smth like that..

Yep, Foldable[NonEmptyMap] does not take into account key :

override def foldLeft[A, B](fa: NonEmptyMap[K, A], b: B)(f: (B, A) => B): B 

btw. standard scala map does it correctly

@travisbrown
Copy link
Contributor

This seems like a good argument against this newtype encoding (which to be honest I've always hated, so this feels kind of vindicating).

I think the issue is Foldable syntax, though, not the Order instance for Int:

scala> import cats.data.NonEmptyMap
import cats.data.NonEmptyMap

scala> import cats.instances.int._
import cats.instances.int._

scala> val nem: NonEmptyMap[Int, String] = NonEmptyMap.of(1 -> "one", 2 -> "two")
nem: cats.data.NonEmptyMap[Int,String] = Map(1 -> one, 2 -> two)

scala> nem.find(_ == "one")
res0: Option[(Int, String)] = Some((1,one))

scala> import cats.syntax.foldable._
import cats.syntax.foldable._

scala> nem.find(_ == "one")
res1: Option[String] = Some(one)

scala> import NonEmptyMap._
import NonEmptyMap._

scala> nem.find(_ == "one")
res2: Option[(Int, String)] = Some((1,one))

I think we're stuck with this (at least until Cats 3), since the imported Foldable syntax will always outrank the NonEmptyMapOps syntax in implicit scope. The best workaround is probably to import NonEmptyMap._ as in my code above.

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

No branches or pull requests

3 participants