-
Notifications
You must be signed in to change notification settings - Fork 6
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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))) | ||
|
||
(s/def ::real | ||
(s/spec real? | ||
:gen #(gen/double* {:infinite? false :NaN? false}))) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what do you think about the name There was a problem hiding this comment. Choose a reason for hiding this commentThe 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" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll mull over 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I understand this comment fully; |
||
|
||
(defn decimal-pred | ||
[precision scale] | ||
(fn [d] | ||
(let [d (bigdec d)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would use bare keys here
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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))))) |
There was a problem hiding this comment.
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?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely