improve handling JSRatio as number in API and conversions

anteoas · Apr 3, 2024 · be80650 · be80650
1 parent 65dc71b
commit be80650
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 30 deletions.
diff --git a/dev/user.clj b/dev/user.clj
@@ -1,7 +1,8 @@
 (ns user
   (:require [cider.piggieback]
             [cljs.repl.browser :as browser]
-            [clojure.java.shell :as sh]))
+            [clojure.java.shell :as sh]
+            [broch.core]))
 
 (defmacro capture-env
   "Capture local bindings.

diff --git a/src/broch/core.cljc b/src/broch/core.cljc
@@ -2,7 +2,7 @@
   (:refer-clojure :exclude [* + - / < <= > >= max min num symbol])
   (:require [broch.impl :as impl]
             [broch.extensions]
-            [broch.numbers :refer [add sub mul div neg]]
+            [broch.numbers :as nums :refer [add sub mul div neg]]
             [broch.protocols :as p]))
 
 ;;
@@ -16,15 +16,15 @@
 
 (defn measure
   "What this quantity is a measure of."
-  [q] (when-not (number? q) (p/measure q)))
+  [q] (when-not (nums/number? q) (p/measure q)))
 
 (defn symbol
   "The unit symbol for this quantity."
-  [q] (when-not (number? q) (p/symbol q)))
+  [q] (when-not (nums/number? q) (p/symbol q)))
 
 (defn composition
   "The composition of this quantity."
-  [q] (when-not (number? q) (p/composition q)))
+  [q] (when-not (nums/number? q) (p/composition q)))
 
 (defn compatible-units
   "Returns units with the same measure as quantity."
@@ -33,17 +33,21 @@
 
 (defn num
   "Get the number from a quantity. Pass through if already a number."
-  [q] (if (number? q) q (p/number q)))
+  [q] (if (nums/number? q) q (p/number q)))
 
 (defn with-num
   "Make copy of a quantity with a different number."
   [quantity n] (impl/quantity quantity n))
 
 (defn boxed
-  "Transform the quantity's number by any fn (i.e. fmap on the quantity-functor).
-  Also works for numbers."
+  "Transform the quantity's number by any fn (i.e. fmap on the quantity-functor). Also works for numbers.
+
+  Beware:
+  1. The return value is not checked, and quantities with non-numeric values will cause parts of broch to break.
+  2. In cljs, broch quantities will sometimes have a ratio number type that is not a regular js Number.
+     These will fail with clojure.core arithmetic ops (+,-,*,/), so use the ops from broch.core instead."
   [f x]
-  (if (number? x)
+  (if (nums/number? x)
     (f x)
     (impl/boxed f x)))
 
@@ -143,9 +147,9 @@
   "Register a new type of unit with the given measure, symbol, composition and/or scaling.
   Returns a fn (fn [number]) that creates a quantity of this unit."
   ([measure symb scale-or-comp]
-   {:pre  [(keyword? measure) (string? symb) (or (number? scale-or-comp) (map? scale-or-comp))]
+   {:pre  [(keyword? measure) (string? symb) (or (nums/number? scale-or-comp) (map? scale-or-comp))]
     :post [(fn? %)]}
-   (let [composition (if (number? scale-or-comp) {:broch/scaled scale-or-comp} scale-or-comp)
+   (let [composition (if (nums/number? scale-or-comp) {:broch/scaled scale-or-comp} scale-or-comp)
          unit        (impl/unit measure symb composition)]
      (impl/register-unit! unit)
      (fn

diff --git a/src/broch/impl.cljc b/src/broch/impl.cljc
@@ -1,11 +1,10 @@
 (ns broch.impl
-  (:refer-clojure :exclude [* add - / symbol read-string rationalize])
+  (:refer-clojure :exclude [* add - / symbol read-string rationalize number? ratio?])
   (:require [broch.protocols :refer [IQuantity composition measure number symbol]]
-            [broch.numbers :refer [add sub mul div neg] :as nums]
+            [broch.numbers :refer [add sub mul div neg number? ratio?] :as nums]
             #?@(:cljs [[cljs.reader]
                        [cljs.compiler]]))
-  #?(:clj (:import (clojure.lang IMeta IObj Obj)
-                   (java.io Writer))))
+  #?(:clj (:import (java.io Writer))))
 
 (defn quantity? [x] (satisfies? IQuantity x))
 (defn same-measure? [x y] (and (quantity? x) (quantity? y) (= (measure x) (measure y))))
@@ -39,7 +38,7 @@
        IEquiv
        (-equiv [this other] (quantities-equal? this other))
        IHash
-       (-hash [this] (hash {:measure -measure :symbol -symbol :number -number}))
+       (-hash [_] (hash {:measure -measure :symbol -symbol :number -number}))
        IComparable
        (-compare [this other] (compare-quantities this other))])
 
@@ -53,7 +52,6 @@
       -composition)))
 
 (defn- quantity* [unit n] (->Quantity (measure unit) (symbol unit) (composition unit) n))
-(defn boxed [f q] (quantity* q (f (number q))))
 (defn simple? [q] (= 1 (get (dissoc (composition q) :broch/scaled) (measure q))))
 
 (defn- safe-scale [n m] (when (and n m) (mul n m)))
@@ -64,15 +62,15 @@
 (defn- convert [a b]
   (let [converted (<-base b (->base a))]
     (cond
-      #?(:clj (ratio? (number a)) :cljs nil) converted
+      (ratio? (number a)) converted
       (nil? (number a)) converted
       (float? (number a)) (quantity* converted (double (number converted)))
       :else (quantity* converted (number converted)))))
 
 (defn- converting-op [unit a b op]
   (let [converted (<-base unit (op (->base a) (->base b)))]
     (cond
-      #?(:clj (ratio? (number a)) :cljs nil) converted
+      (ratio? (number a)) converted
       (float? (number a)) (quantity* converted (double (number converted)))
       :else (quantity* converted (number converted)))))
 
@@ -84,7 +82,7 @@
     (nil? x)
     unit
 
-    (or (number? x) (nums/ratio? x))
+    (number? x)
     (quantity* unit x)
 
     (string? x)
@@ -101,6 +99,8 @@
 
     :else (throw (ex-info (str "Unhandled case: " x " " (type x)) {:unit unit :x x}))))
 
+(defn boxed [f q] (quantity q (f (number q))))
+
 (defn- pow [n x]
   (if (neg? x)
     (div 1 (reduce mul (repeat (abs x) n)))

diff --git a/src/broch/numbers.cljc b/src/broch/numbers.cljc
@@ -1,11 +1,10 @@
 (ns broch.numbers
-  (:refer-clojure :exclude [bigint abs ratio? rationalize integer?])
+  (:refer-clojure :exclude [bigint abs ratio? rationalize integer? number?])
   (:require #?@(:clj  [[clojure.core :as core]]
                 :cljs [[cljs.core :as core]
                        [cljs.math :as math]
-                       [cljs.pprint]])
-            [clojure.string :as str])
-  #?(:clj (:import (clojure.lang Ratio))))
+                       [cljs.pprint]
+                       [clojure.string :as str]])))
 
 (defn bigint [n]
   #?(:clj  (core/bigint n)
@@ -71,6 +70,9 @@
   #?(:clj (core/ratio? x)
      :cljs (instance? JSRatio x)))
 
+(defn number? [x]
+  (or (clojure.core/number? x) (ratio? x)))
+
 #?(:cljs
    (defn- do-in-common-ratio [a b op]
      (let [[na da] [(bigint (numer a)) (bigint (denom a))]
@@ -147,7 +149,7 @@
 #?(:cljs (defn js-ratio->number [^JSRatio r] (/ (js/Number (numer r)) (js/Number (denom r)))))
 
 (defn weird? [n]
-  (or (and (number? n) (NaN? n)) (infinite? n)))
+  (or (and (clojure.core/number? n) (NaN? n)) (infinite? n)))
 
 (defn downcast
   "Downcast if possible without losing precision."

diff --git a/test/broch/core_test.cljs b/test/broch/core_test.cljs
@@ -101,11 +101,20 @@
 
 (declare thrown?)
 (deftest number-handling
-  (is (not (b/quantity? 123)))
-  (is (nil? (b/measure 123)))
-  (is (nil? (b/symbol 123)))
-  (is (= 123 (b/num 123)))
-  (is (thrown? js/Error (b/with-num 1 123))))
+  (testing "regular numbers"
+    (is (not (b/quantity? 123)))
+    (is (nil? (b/measure 123)))
+    (is (nil? (b/symbol 123)))
+    (is (= 123 (b/num 123)))
+    (is (thrown? js/Error (b/with-num 1 123)))
+    (is (= 2 (b/boxed #(b/+ % 1) 1))))
+  (testing "JSRatios"
+    (is (not (b/quantity? (b// 1 3))))
+    (is (nil? (b/measure (b// 1 3))))
+    (is (nil? (b/symbol (b// 1 3))))
+    (is (= (b// 1 3) (b/num (b// 1 3))))
+    (is (thrown? js/Error (b/with-num 1 (b// 1 3))))
+    (is (= (b// 4 3) (b/boxed #(b/+ % 1) (b// 1 3))))))
 
 (deftest nil-handling
   (is (thrown? js/Error (b/measure nil)))