twitter / algebird

Abstract Algebra for Scala
https://twitter.github.io/algebird
Apache License 2.0
2.29k stars 346 forks source link

Tuple2 implicits do not work with Arrays #661

Open tovbinm opened 6 years ago

tovbinm commented 6 years ago

The following code fails to compile:

import com.twitter.algebird.Monoid._
import com.twitter.algebird.Operators._
val m = Map.empty[Int, (Array[Long], Array[Long])]
m + m // fails to compile

unless we explicitly add:

implicit val sgTuple2 = new com.twitter.algebird.Tuple2Semigroup[Array[Long], Array[Long]]()

Tested with Algebird 0.13.4 and Scala 2.11.12

tovbinm commented 6 years ago

In fact, same happens with simple tuples as well:

import com.twitter.algebird.Monoid._
import com.twitter.algebird.Operators._
val x = 1 -> 1
x + x // fails to compile
johnynek commented 6 years ago

Huh. This is very strange. Can you make a PR with a test to demonstrate the failure.

We use tuples with Semigroups all the time.

Why are you importing Monoid._? I wonder if you are setting up a conflicting implicit and scala just silently fails when two implicits can be found.

tovbinm commented 6 years ago

The Monoid._ is mainly needed because of the ArrayMonoid

tovbinm commented 6 years ago

Here you go - https://github.com/tovbinm/algebird/commit/776cfe12f23e78cf7b90855427dc656049a8030d

tovbinm commented 6 years ago

Also, I think I found what's the problem - import order matters (at least in the test): This would fail:

import com.twitter.algebird.Monoid._
import com.twitter.algebird.Operators._

This works:

import com.twitter.algebird.Operators._
import com.twitter.algebird.Monoid._

But in my project import order does not matter, so the problem persists.

tovbinm commented 6 years ago

Here is the failure again in Ammonite:

Welcome to the Ammonite Repl 1.1.0
(Scala 2.12.4 Java 1.8.0_131)
If you like Ammonite, please support our development at www.patreon.com/lihaoyi
@ import $ivy.`com.twitter::algebird-core:0.13.4`
import $ivy.$

@ import com.twitter.algebird.Operators._
import com.twitter.algebird.Operators._

@ import com.twitter.algebird.Monoid._
import com.twitter.algebird.Monoid._

@ Array(1,2) -> Array(1,2) + Array(1,2) -> Array(1,2)
cmd3.sc:1: diverging implicit expansion for type algebra.Monoid[(Array[Int], Array[Int])]
starting with method fromAlgebraAdditiveMonoid in trait FromAlgebraMonoidImplicit1
val res3 = Array(1,2) -> Array(1,2) + Array(1,2) -> Array(1,2)
                      ^
cmd3.sc:1: value + is not a member of (Array[Int], Array[Int])
val res3 = Array(1,2) -> Array(1,2) + Array(1,2) -> Array(1,2)
                                    ^
Compilation Failed
johnynek commented 6 years ago

But scala should search the Monoid companion. What if you don’t try the Monoid import?

On Sun, Jul 29, 2018 at 05:45 Matthew Tovbin notifications@github.com wrote:

here is it in Ammonite:

Welcome to the Ammonite Repl 1.1.0 (Scala 2.12.4 Java 1.8.0_131) If you like Ammonite, please support our development at www.patreon.com/lihaoyi @ import $ivy.com.twitter::algebird-core:0.13.4 import $ivy.$

@ import com.twitter.algebird.Operators. import com.twitter.algebird.Operators.

@ import com.twitter.algebird.Monoid. import com.twitter.algebird.Monoid.

@ Array(1,2) + Array(1,1)cmd3.sc:1: diverging implicit expansion for type algebra.Monoid[Array[Int]] starting with method fromAlgebraAdditiveMonoid in trait FromAlgebraMonoidImplicit1 val res3 = Array(1,2) + Array(1,1) ^cmd3.sc:1: value + is not a member of Array[Int] val res3 = Array(1,2) + Array(1,1) ^ Compilation Failed

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/twitter/algebird/issues/661#issuecomment-408686400, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEJdjE0u-uFH2Yt1D3JiclwQfVqsNrEks5uLdirgaJpZM4VlNdl .

-- P. Oscar Boykin, Ph.D. | http://twitter.com/posco | http://pobox.com/~boykin

tovbinm commented 6 years ago

Well, it assumes String in + operator then:

Welcome to the Ammonite Repl 1.1.0
(Scala 2.12.4 Java 1.8.0_131)
If you like Ammonite, please support our development at www.patreon.com/lihaoyi
@ import $ivy.`com.twitter::algebird-core:0.13.4`
import $ivy.$

@ import com.twitter.algebird._
import com.twitter.algebird._

@ import com.twitter.algebird.Operators._
import com.twitter.algebird.Operators._

@ (Array(1,2) -> Array(1,2)) + (Array(1,2) -> Array(1,2))
cmd3.sc:1: type mismatch;
 found   : (Array[Int], Array[Int])
 required: String
val res3 = (Array(1,2) -> Array(1,2)) + (Array(1,2) -> Array(1,2))
                                                    ^
Compilation Failed
johnynek commented 6 years ago

Honestly it’s not that common to use Operators with Algebird.

I can look into it, but in the meantime I recommend you use Semigroup.plus on the companion.

It is probably an ambiguous implicit somewhere.