Scala 2.12 support #16
Conversation
Latest versions of scalacheck got stricter and would fail for Gen.choose(0, SOME NEGATIVE NUMBER) I found this problem while upgrading bonsai to Scala 2.12.
I bumped scalatest and scalacheck to versions that support Scala 2.12 (in addition to Scala 2.10 and 2.11).
| @@ -18,7 +18,7 @@ class IndexedBitSetSpec extends WordSpec with Matchers with Checkers { | |||
|
|
|||
| implicit val arbitraryBitSetWithIndex: Arbitrary[BitSetWithIndex] = | |||
| Arbitrary(for { | |||
| len <- arbitrary[Short].map(_.toInt) | |||
| len <- arbitrary[Short].map(_.toInt) if len > 0 | |||
|
LGTM! |
| @@ -18,7 +18,7 @@ class IndexedBitSetSpec extends WordSpec with Matchers with Checkers { | |||
|
|
|||
| implicit val arbitraryBitSetWithIndex: Arbitrary[BitSetWithIndex] = | |||
| Arbitrary(for { | |||
| len <- arbitrary[Short].map(_.toInt) | |||
| len <- arbitrary[Short].map(_.toInt) if len >= 0 | |||
There was a problem hiding this comment.
Can we instead change this to something like the following?
arbitrary[Short].map(_ & 0xffff)This way we get a non-negative value without any filtering (which I think is best to avoid when possible).
There was a problem hiding this comment.
don't you mean _ & 0x7fff to make sure it is >=0?
There was a problem hiding this comment.
0xffff is a positive Int (65535); that map changes the type as well as the value.
There was a problem hiding this comment.
ahh, yeah. We are resulting in an Int.
So, in that case, I don't like _ & 0xffff because all negative numbers are mapped to 65535, so half the time we might see a really big list. I think we are better off with:
x => if(x < 0) -(x.toInt) else x
There was a problem hiding this comment.
@johnynek That's not quite true either.
scala> 0xffff
res0: Int = 65535
scala> Short.MinValue & 0xffff
res1: Int = 32768
scala> (Short.MinValue + 100) & 0xffff
res2: Int = 32868
scala> (Short.MinValue + 10000) & 0xffff
res3: Int = 42768
But I am fine with any of these approaches that remove the if.
There was a problem hiding this comment.
yeah, I'm an idiot. Sorry for the goof up. It should be uniformly distributed if the input is uniform.
|
Given this lgtmed, I'll go ahead and merge. |
Check individual commits for details.
I tested it by running
+ test(that runs tests for all Scala versions).