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

Added NotNaN predicate #596

Merged
merged 3 commits into from
Dec 13, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,7 @@ The library comes with these predefined predicates:
* `NonDivisible[N]`: checks if an integral value is not evenly divisible by `N`
* `Even`: checks if an integral value is evenly divisible by 2
* `Odd`: checks if an integral value is not evenly divisible by 2
* `NotNaN`: checks if a floating-point number is not NaN

[`string`](https://github.com/fthomas/refined/blob/master/modules/core/shared/src/main/scala/eu/timepit/refined/string.scala)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ object numeric extends NumericInference {
/** Predicate that checks if an integral value modulo `N` is `O`. */
final case class Modulo[N, O](n: N, o: O)

/** Predicate that checks if a floating-point number value is not NaN. */
case class NotNaN()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be final.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other negated predicates use Non as prefix, like NonEmpty, NonPositive, NonNegative, or NonDivisible. I think we should do it here too and rename it to NonNaN. It seems to be a common rule to prefix with non- to negate the meaning of a word.


/** Predicate that checks if a numeric value is less than or equal to `N`. */
type LessEqual[N] = Not[Greater[N]]

Expand Down Expand Up @@ -117,6 +120,14 @@ object numeric extends NumericInference {
Modulo(wn.fst, wo.fst)
)
}

object NotNaN {
implicit def floatNotNaNValidate: Validate.Plain[Float, NotNaN] = fromIsNaN(_.isNaN)
implicit def doubleNotNaNValidate: Validate.Plain[Double, NotNaN] = fromIsNaN(_.isNaN)

def fromIsNaN[A](isNaN: A => Boolean): Validate.Plain[A, NotNaN] =
Validate.fromPredicate(x => !isNaN(x), x => s"$x != NaN", NotNaN())
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be parenthesis in the string representation of this predicate.

Suggested change
Validate.fromPredicate(x => !isNaN(x), x => s"$x != NaN", NotNaN())
Validate.fromPredicate(x => !isNaN(x), x => s"($x != NaN)", NotNaN())

Other predicates do this, too.

}
}

private[refined] trait NumericInference {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package eu.timepit.refined.predicates

object all extends AllPredicates with AllPredicatesBinCompat1
object all extends AllPredicates with AllPredicatesBinCompat1 with AllPredicatesBinCompat2

trait AllPredicates
extends BooleanPredicates
Expand All @@ -11,3 +11,5 @@ trait AllPredicates
with StringPredicates

trait AllPredicatesBinCompat1 extends StringPredicatesBinCompat1

trait AllPredicatesBinCompat2 extends NumericPredicatesBinCompat1
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package eu.timepit.refined.predicates

import eu.timepit.refined

object numeric extends NumericPredicates
object numeric extends NumericPredicates with NumericPredicatesBinCompat1

trait NumericPredicates {
final type Less[N] = refined.numeric.Less[N]
Expand Down Expand Up @@ -36,3 +36,8 @@ trait NumericPredicates {

final val Interval = refined.numeric.Interval
}

trait NumericPredicatesBinCompat1 {
final type NotNaN = refined.numeric.NotNaN
final val NotNaN = refined.numeric.NotNaN
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package eu.timepit.refined.types

/** Module for all predefined refined types. */
object all extends AllTypes with AllTypesBinCompat1
object all extends AllTypes with AllTypesBinCompat1 with AllTypesBinCompat2

trait AllTypes
extends CharTypes
Expand All @@ -12,3 +12,5 @@ trait AllTypes
with TimeTypes

trait AllTypesBinCompat1 extends NumericTypesBinCompat1

trait AllTypesBinCompat2 extends NumericTypesBinCompat2
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package eu.timepit.refined.types

import eu.timepit.refined.api.{Refined, RefinedTypeOps}
import eu.timepit.refined.numeric.{Negative, NonNegative, NonPositive, Positive}
import eu.timepit.refined.numeric.{Negative, NonNegative, NonPositive, NotNaN, Positive}

/** Module for numeric refined types. */
object numeric {
Expand Down Expand Up @@ -165,6 +165,16 @@ object numeric {
type NonPosBigDecimal = BigDecimal Refined NonPositive

object NonPosBigDecimal extends RefinedTypeOps[NonPosBigDecimal, BigDecimal]

/** A `Float` that is not NaN. */
type FloatNotNaN = Float Refined NotNaN
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we have NonNegInt or PosDouble, this should be NonNaNFloat.


object FloatNotNaN extends RefinedTypeOps[FloatNotNaN, Float]

/** A `Double` that is not NaN. */
type DoubleNotNaN = Double Refined NotNaN

object DoubleNotNaN extends RefinedTypeOps[DoubleNotNaN, Double]
}

trait NumericTypes {
Expand Down Expand Up @@ -266,3 +276,11 @@ trait NumericTypesBinCompat1 {
final type NonPosBigDecimal = numeric.NonPosBigDecimal
final val NonPosBigDecimal = numeric.NonPosBigDecimal
}

trait NumericTypesBinCompat2 {
final type FloatNotNaN = numeric.FloatNotNaN
final val FloatNotNaN = numeric.FloatNotNaN

final type DoubleNotNaN = numeric.DoubleNotNaN
final val DoubleNotNaN = numeric.DoubleNotNaN
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package eu.timepit.refined

import eu.timepit.refined.TestUtils._
import eu.timepit.refined.numeric._
import org.scalacheck.{Arbitrary, Gen, Properties}
import org.scalacheck.Prop._
import org.scalacheck.Properties
import shapeless.nat._

class NumericValidateSpec extends Properties("NumericValidate") {
Expand Down Expand Up @@ -179,4 +179,24 @@ class NumericValidateSpec extends Properties("NumericValidate") {
val s = showExpr[Interval.Closed[_0, _1]](0.5)
(s ?= "(!(0.5 < 0) && !(0.5 > 1))") || (s ?= "(!(0.5 < 0.0) && !(0.5 > 1.0))")
}

val floatWithNaN: Gen[Float] = Gen.frequency(8 -> Arbitrary.arbitrary[Float], 2 -> Float.NaN)
val doubleWithNaN: Gen[Double] = Gen.frequency(8 -> Arbitrary.arbitrary[Double], 2 -> Double.NaN)

property("NotNaN.Float.isValid") = forAll(floatWithNaN) { (d: Float) =>
isValid[NotNaN](d) ?= !d.isNaN
}

property("NotNaN.Float.showExpr") = secure {
showExpr[NotNaN](Float.NaN) ?= "NaN != NaN"
}

property("NotNaN.Double.isValid") = forAll(doubleWithNaN) { (d: Double) =>
isValid[NotNaN](d) ?= !d.isNaN
}

property("NotNaN.Double.showExpr") = secure {
showExpr[NotNaN](Double.NaN) ?= "NaN != NaN"
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ object all
with CharInstances
with GenericInstances
with NumericInstances
with NumericInstancesBinCompat1
with RefTypeInstances
with StringInstances
with StringInstancesBinCompat1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import org.scalacheck.Gen.Choose
* Module that provides `Arbitrary` instances and generators for
* numeric predicates.
*/
object numeric extends NumericInstances
object numeric extends NumericInstances with NumericInstancesBinCompat1

trait NumericInstances {

Expand Down Expand Up @@ -111,3 +111,15 @@ trait NumericInstances {
): Arbitrary[F[T, P]] =
arbitraryRefType(Gen.chooseNum(min, max))
}

trait NumericInstancesBinCompat1 {
implicit def floatNotNaNArbitrary[F[_, _]: RefType](
implicit arb: Arbitrary[Float]
): Arbitrary[F[Float, NotNaN]] =
arbitraryRefType(arb.arbitrary.suchThat(x => !x.isNaN))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nitpick: Using suchThat could lead in theory to "discarded too many values" errors. I don't think this will ever be a problem in this case but we could eliminate this possibility altogether by mapping NaN values to 0.0.

Suggested change
arbitraryRefType(arb.arbitrary.suchThat(x => !x.isNaN))
arbitraryRefType(arb.arbitrary.map(x => if (x.isNaN) 0.0F else x))


implicit def doubleNotNaNArbitrary[F[_, _]: RefType](
implicit arb: Arbitrary[Double]
): Arbitrary[F[Double, NotNaN]] =
arbitraryRefType(arb.arbitrary.suchThat(x => !x.isNaN))
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import eu.timepit.refined.W
import eu.timepit.refined.api.Refined
import eu.timepit.refined.numeric._
import eu.timepit.refined.scalacheck.numeric._
import eu.timepit.refined.types.numeric.{NegDouble, NonNegInt, NonNegLong, PosFloat}
import eu.timepit.refined.types.numeric._
import eu.timepit.refined.types.time.Minute
import org.scalacheck.Prop._
import org.scalacheck.Properties
Expand Down Expand Up @@ -36,6 +36,10 @@ class NumericArbitrarySpec extends Properties("NumericArbitrary") {

property("GreaterEqual[_10]") = checkArbitraryRefinedType[Int Refined GreaterEqual[_10]]

property("FloatNotNaN") = checkArbitraryRefinedType[FloatNotNaN]

property("DoubleNotNaN") = checkArbitraryRefinedType[DoubleNotNaN]

property("PosFloat") = checkArbitraryRefinedType[PosFloat]

property("NonPositive") = checkArbitraryRefinedType[Short Refined NonPositive]
Expand Down