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

Add schpec.number #1

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
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
119 changes: 119 additions & 0 deletions src/com/gfredericks/schpec/number.clj
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
(ns com.gfredericks.schpec.number
(:require [clojure.spec :as s]
[clojure.spec.gen :as gen])
(:import [java.math BigDecimal MathContext RoundingMode]))

(defn real?
[x]
(and (number? x)
(case x
Double/POSITIVE_INFINITY false
Double/NEGATIVE_INFINITY false
Double/NaN false
true)))
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think this case statement works; could you add tests?

Copy link
Author

Choose a reason for hiding this comment

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

Absolutely


(s/def ::real
(s/spec real?
:gen #(gen/double* {:infinite? false :NaN? false})))
Copy link
Owner

Choose a reason for hiding this comment

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

Is this spec/pred intended to apply just to doubles? If so, I would use a name closer to "double" or "floating-point", and check that it's an instance of Double in the predicate; I personally don't like the name "real" for numeric types because of the mismatch with the mathematical term.

Copy link
Author

Choose a reason for hiding this comment

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

No, it's intended to apply to all real number values, which as far as I can reason, include all things clojure considers numbers except these three corner cases. I confess, I don't see any mismatch between this predicate and the mathematical domain of real numbers. Is it because there are irrational numbers (e.g. pi) that don't have precise representations?

Clojure core already provides double?. My intent here is to provide predicates and specs for numbers in terms of their domain (e.g. real numbers, numbers that can be precisely represented as decimals with such and such precision), not their actual types.

Copy link
Owner

Choose a reason for hiding this comment

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

what do you think about the name finite? for this?

Copy link
Owner

Choose a reason for hiding this comment

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

yes, irrational numbers are the sort of thing I'm thinking of w.r.t. the word "real"

Copy link
Author

Choose a reason for hiding this comment

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

I'll mull over finite?. Note of course there are plenty of real, irrational numbers that do pass this predicate: (/ 1 3) and friends. It's not pi's fault that clojure doesn't have an e.g. Taylor series expansion type to compute it to arbitrary precisions :)

If the predicate failed to cover such a case, I'd buy the argument that it's not named correctly. But AFAICT, it covers all real numbers expressible in clojure's arithmetic.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure I understand this comment fully; (/ 1 3) looks like a rational.


(defn decimal-pred
[precision scale]
(fn [d]
(let [d (bigdec d)]
Copy link
Owner

Choose a reason for hiding this comment

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

by doing this coercion are you aiming to let the spec match floating point numbers or integers? I usually prefer specs that lock down a specific type (like BigDecimal in this case) as it makes it easier to handle and test things when there aren't several possibilities.

Copy link
Author

Choose a reason for hiding this comment

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

I think that case would be nicely handled by a union predicate or spec.

(and (or (not precision)
(>= precision (.precision d)))
(or (not scale)
(let [d-scale (.scale d)]
(and (not (neg? d-scale))
(>= scale d-scale))))))))

(defn decimal-in
"Specs a decimal number. The number type may be anything that bigdec
accepts. Options:

:precision - the number of digits in the unscaled value (default none)
:scale - the number of digits to the right of the decimal (default none)
:min - minimum value (inclusive, default none)
:max - maximum value (inclusive, default none)

A decimal satifies this spec if its precision and scale are not greater
than the specified precision and scale, if given.

Note that the java math definition of precision and scale may not be the
same as e.g. your database. For example, -1E-75M has a precision of 1 and a
scale of 75. For sanest results, you should specify both, though the spec
does not require both."
[& options]
(let [{:keys [precision scale min max]} options
dec-pred (decimal-pred precision scale)]
(letfn [(pred [d]
(and (dec-pred d)
(or (not min)
(>= d min))
(or (not max)
(>= max d))))
(gen []
(let [min (or min
(and precision
(-> BigDecimal/ONE
(.movePointRight precision)
dec
.negate)))
max (or max
(and precision
(-> BigDecimal/ONE
(.movePointRight precision)
dec)))
mc (when precision
(MathContext. precision RoundingMode/HALF_UP))]
(letfn [(f [d]
(cond-> (bigdec d)
scale
(.setScale scale BigDecimal/ROUND_HALF_UP)
precision
(.round mc)))]
(gen/fmap f (gen/double* {:infinite? false
:NaN? false
:min min
:max max})))))]
(s/spec pred :gen gen))))

(s/def :specs.number.decimal/precision
Copy link
Owner

Choose a reason for hiding this comment

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

you're putting these in a different namespace to distinguish them from the user-facing specs?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, they're not intended as published specs for general use, they're just to support the s/keys :opt-un. I've tried a few different techniques for bespoke options specs and haven't really found anything I like better than this yet.

Copy link
Owner

Choose a reason for hiding this comment

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

that's interesting, and it raises the question of how users are going to be discovering available specs; if they just rely on our documentation, then this isn't a concern.

Copy link

Choose a reason for hiding this comment

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

I would use bare keys here

(s/def ::plugin-item
  (s/and
   (s/cat :lib-name symbol?
          :version-str non-blank-string?
          :dependency-item-args
          (s/* (s/alt :optional      (s/cat :ky #{:optional}
                                            :val boolean?)
                      :scope         (s/cat :ky #{:scope}
                                            :val ::name-like)
                      :classifier    (s/cat :ky #{:classifier}
                                            :val ::name-like)
                      :native-prefix (s/cat :ky #{:native-prefix}
                                            :val (some-fn string? symbol? keyword?))
                      :extension     (s/cat :ky  #{:extension}
                                            :val (some-fn string? symbol? keyword?))
                      :exclusions    (s/cat :ky #{:exclusions}
                                            :val ::exclusions)
                      :plugins-args  (s/cat :ky #{:middleware :hooks}
                                            :val boolean?))))))

Copy link

Choose a reason for hiding this comment

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

It seems strange to register keys that are just argument keys, they are not global types or contracts...

pos-int?)

(s/def :specs.number.decimal/scale
(s/spec (fn [x] (and (int? x) (not (neg? x))))
:gen #(gen/large-integer* {:min 0})))

(s/def :specs.number.decimal/min
::real)

(s/def :specs.number.decimal/max
::real)

(s/fdef decimal-in
:args (s/and (s/keys* :opt-un [:specs.number.decimal/precision
:specs.number.decimal/scale
:specs.number.decimal/min
:specs.number.decimal/max])
#(let [{:keys [min max precision scale]} %
dec-pred (decimal-pred precision scale)]
(and (or (not (and min max))
(>= max min))
(or (not precision)
(pos? precision))
(or (not scale)
(not (neg? scale)))
(or (not (and precision scale))
(>= precision scale))
(or (not min)
(dec-pred min))
(or (not max)
(dec-pred max)))))
:ret s/spec?
:fn #(let [{:keys [ret args]} %
{:keys [min max]} args]
(and (or (not min)
(s/valid? ret min))
(or (not max)
(s/valid? ret max)))))