Add Invariant instances for Numeric and Integral #3773
Conversation
…invariant-numeric
Also add instances to Invariant companion and instances.all.
| def toLong(x: B): Long = fa.toLong(g(x)) | ||
| def toFloat(x: B): Float = fa.toFloat(g(x)) | ||
| def toDouble(x: B): Double = fa.toDouble(g(x)) | ||
| def parseString(str: String): Option[B] = fa.parseString(str).map(f) |
There was a problem hiding this comment.
This differs from the 2.12 implementation because parseString was added in 2.13.
| Eq[Band[Set[Boolean]]] | ||
| cats.laws.discipline.ExhaustiveCheck[Set[Boolean]] | ||
| Eq[(Set[Boolean], Boolean)] | ||
| Eq[(Set[Boolean], Set[Boolean] => (Set[Boolean], Boolean))] | ||
| Eq[CommutativeSemigroup[Set[Boolean]]] |
There was a problem hiding this comment.
There was a question about these lines in the previous PR. I've removed them but happy to revert if they are necessary.
There was a problem hiding this comment.
They might be compilation tests, no?
There was a problem hiding this comment.
On second thought, they probably don't belong there at all. So let's keep them removed.
| // These allow us to capture the cases where operations on Numeric throw (eg when causing an overflow). These are | ||
| // represented by `None` and allow us to compare two Numeric instances, verifying that when one throws, the other | ||
| // also throws. | ||
| def makeUnaryFnSafe[X, R](f: X => R): X => Option[R] = | ||
| x => Either.catchOnly[IllegalArgumentException](f(x)).toOption | ||
| def makeBinaryFnSafe[X, Y, R](f: (X, Y) => R): (X, Y) => Option[R] = | ||
| (x, y) => Either.catchOnly[IllegalArgumentException](f(x, y)).toOption |
There was a problem hiding this comment.
Note that here and in a couple of other places, I've had to find a way of reconciling the unsafe nature of the methods on Numeric with defining an Eq.
My approach has been to require that for two Numeric instances to be equal, when one throws the other throws. Lifting the values into Option and using None to represent cases that throw erases the difference between different types of thrown exceptions. I've decided that this is okay given the alternative would be a dirty Eq for ArithmaticException and IllegalArgumentException.
Happy to change this approach to something else.
There was a problem hiding this comment.
Actually I think this is unnecessary except in the fromInt case. I will rework it...
|
The test failure looks to be unrelated to my changes. |
Probably a flaky test. |
Fixes #3220.
This PR continues the work done in #3222 by @kubukoz. My additional work is:
InvariantcompanionAllInstances2.12and2.13+by handling theparseStringmethod that was added toNumericin2.13Invariant[Integral]Note that I haven't added
Invariant[Fractional]because the laws testing looks to be a bit messy. I might think a bit more on how to test such an instance and add it in a separate PR.