From 9d2b6da1cc7364ecd1228fcc1aeaa5d92d704a7b Mon Sep 17 00:00:00 2001 From: Simon Brooke Date: Wed, 26 Jun 2019 09:37:00 +0100 Subject: [PATCH 1/5] Started work on *safe-sparse-operations* Not fully working yet --- src/sparse_array/core.clj | 47 ++++++++++++++++++++++++--------- test/sparse_array/core_test.clj | 34 +++++++++++++++++++----- 2 files changed, 63 insertions(+), 18 deletions(-) diff --git a/src/sparse_array/core.clj b/src/sparse_array/core.clj index b169df8..6906a36 100644 --- a/src/sparse_array/core.clj +++ b/src/sparse_array/core.clj @@ -1,5 +1,7 @@ (ns sparse-array.core) +(def ^:dynamic *safe-sparse-operations* false) + (defn make-sparse-array "Make a sparse array with these `dimensions`. Every member of `dimensions` must be a keyword; otherwise, `nil` will be returned." @@ -18,22 +20,37 @@ "`true` if `x` is a sparse array conforming to the conventions established by this library, else `false`." ([x] - (and - (map? x) - (pos? (:dimensions x)) - (keyword? (:coord x)) - (if - (coll? (:content x)) - (every? - sparse-array? - (map #(x %) (filter integer? (keys x)))) - (= (:content x) :data))))) + (apply + sparse-array? + (cons + x + (cons + (:coord x) + (if + (coll? (:content x)) + (:content x)))))) + ([x & axes] + (and + (map? x) + (pos? (:dimensions x)) + (keyword? (:coord x)) + (= (:coord x) (first axes)) + (if + (rest axes) + (and + (= (:content x) (rest axes)) + (every? + sparse-array? + (map #(x %) (filter integer? (keys x))))) + (= (:content x) :data))))) (defn put "Return a sparse array like this `array` but with this `value` at these `coordinates`. Returns `nil` if any coordinate is invalid." [array value & coordinates] - (if + (cond + (and *safe-sparse-operations* (sparse-array? array)) + (throw (ex-info "Sparse array expected" {:array array})) (every? #(and (integer? %) (or (zero? %) (pos? %))) coordinates) @@ -49,7 +66,13 @@ (or (array (first coordinates)) (apply make-sparse-array (:content array))) - (cons value (rest coordinates)))))))) + (cons value (rest coordinates)))))) + *safe-sparse-operations* + (throw + (ex-info + "Coordinates must be zero or positive integers" + {:coordinates coordinates + :invalid (remove integer? coordinates)})))) (defn get "Return the value in this sparse `array` at these `coordinates`." diff --git a/test/sparse_array/core_test.clj b/test/sparse_array/core_test.clj index 815618f..216681a 100644 --- a/test/sparse_array/core_test.clj +++ b/test/sparse_array/core_test.clj @@ -4,7 +4,21 @@ (deftest creation-and-testing (testing "Creation and testing." - (is (sparse-array? (make-sparse-array :x :y :z))))) + (is (sparse-array? (make-sparse-array :x :y :z))) + (is (sparse-array? {:dimensions 2, + :coord :x, + :content '(:y), + 3 {:dimensions 1, + :coord :y, + :content :data, + 4 "hello"}, + 4 {:dimensions 1, + :coord :y, + :content :data, + 3 "goodbye"}})) + (is-not (sparse-array? [])) + (is-not (sparse-array? "hello")) + )) (deftest put-and-get (testing "get" @@ -84,9 +98,17 @@ [nil nil nil nil nil] [nil nil nil nil "hello"] [nil nil nil "goodbye" nil]] - actual (sparse-to-dense (put - (put - (make-sparse-array :x :y) - "hello" 3 4) - "goodbye" 4 3))] + actual (sparse-to-dense {:dimensions 2, + :coord :x, + :content '(:y), + 3 {:dimensions 1, + :coord :y, + :content :data, + 4 "hello"}, + 4 {:dimensions 1, + :coord :y, + :content :data, + 3 "goodbye"}})] (is (= actual expected))))) + + From dfd5728e57d812c455f726df2a235cb328918529 Mon Sep 17 00:00:00 2001 From: Simon Brooke Date: Wed, 26 Jun 2019 10:51:08 +0100 Subject: [PATCH 2/5] All tests pass again, with much more safety. --- src/sparse_array/core.clj | 7 ++++--- test/sparse_array/core_test.clj | 34 +++++++++++++++++++++++++++++++-- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/sparse_array/core.clj b/src/sparse_array/core.clj index 6906a36..be8a3f0 100644 --- a/src/sparse_array/core.clj +++ b/src/sparse_array/core.clj @@ -32,17 +32,18 @@ ([x & axes] (and (map? x) + (number? (:dimensions x)) (pos? (:dimensions x)) (keyword? (:coord x)) (= (:coord x) (first axes)) (if - (rest axes) + (empty? (rest axes)) + (= (:content x) :data) (and (= (:content x) (rest axes)) (every? sparse-array? - (map #(x %) (filter integer? (keys x))))) - (= (:content x) :data))))) + (map #(x %) (filter integer? (keys x))))))))) (defn put "Return a sparse array like this `array` but with this `value` at these diff --git a/test/sparse_array/core_test.clj b/test/sparse_array/core_test.clj index 216681a..b7d5707 100644 --- a/test/sparse_array/core_test.clj +++ b/test/sparse_array/core_test.clj @@ -16,8 +16,38 @@ :coord :y, :content :data, 3 "goodbye"}})) - (is-not (sparse-array? [])) - (is-not (sparse-array? "hello")) + (is (= (sparse-array? []) false)) + (is (= (sparse-array? "hello") false)) + (is (= + (sparse-array? + (dissoc (make-sparse-array :x :y :z) :dimensions)) + false) + "All mandatory keywords must be present: dimensions") + (is (= + (sparse-array? + (dissoc (make-sparse-array :x :y :z) :coord)) + false) + "All mandatory keywords must be present: coord") + (is (= + (sparse-array? + (dissoc (make-sparse-array :x :y :z) :content)) + false) + "All mandatory keywords must be present: content") + (is (= + (sparse-array? {:dimensions 2, + :coord :x, + :content '(:y), + 3 {:dimensions 1, + :coord :y, + :content :data, + 4 "hello"}, + 4 {:dimensions 1, + :coord :y, + :content :data, + 3 "goodbye"} + 5 :foo}) + false) + "Can't have data in a non-data layer") )) (deftest put-and-get From b7e6c36d68f144b3d3d23c4d6ac991a2af92f977 Mon Sep 17 00:00:00 2001 From: Simon Brooke Date: Wed, 26 Jun 2019 12:06:22 +0100 Subject: [PATCH 3/5] More sophisticated sanity tests all working satisfactorily --- project.clj | 19 ++++++++- src/sparse_array/core.clj | 73 +++++++++++++++++++++++++++------ test/sparse_array/core_test.clj | 43 +++++++++++++++++++ 3 files changed, 122 insertions(+), 13 deletions(-) diff --git a/project.clj b/project.clj index d9c47b7..fb40d19 100644 --- a/project.clj +++ b/project.clj @@ -3,4 +3,21 @@ :url "http://example.com/FIXME" :license {:name "Eclipse Public License" :url "http://www.eclipse.org/legal/epl-v10.html"} - :dependencies [[org.clojure/clojure "1.8.0"]]) + :dependencies [[org.clojure/clojure "1.8.0"]] + + ;; `lein release` doesn't play nice with `git flow release`. Run `lein release` in the + ;; `develop` branch, then merge the release tag into the `master` branch. + + :release-tasks [["vcs" "assert-committed"] + ["clean"] + ["test"] + ["codox"] + ["change" "version" "leiningen.release/bump-version" "release"] + ["vcs" "commit"] + ;; ["vcs" "tag"] -- not working, problems with secret key + ["uberjar"] + ["install"] + ["deploy" "clojars"] + ["change" "version" "leiningen.release/bump-version"] + ["vcs" "commit"]]) + diff --git a/src/sparse_array/core.clj b/src/sparse_array/core.clj index be8a3f0..ca98565 100644 --- a/src/sparse_array/core.clj +++ b/src/sparse_array/core.clj @@ -1,6 +1,15 @@ (ns sparse-array.core) -(def ^:dynamic *safe-sparse-operations* false) +(def ^:dynamic *safe-sparse-operations* + "Whether spase array operations should be conducted safely, with careful + checking of data conventions and exceptions thrown if expectations are not + met. Normally `false`." + false) + +(defmacro unsafe-sparse-operations? + "returns `true` if `*safe-sparse-operations*` is `false`, and vice versa." + [] + (not (true? *safe-sparse-operations*))) (defn make-sparse-array "Make a sparse array with these `dimensions`. Every member of `dimensions` @@ -16,6 +25,18 @@ :data (rest dimensions))})) +(defn- safe-test-or-throw + "If `v` is truthy or `*safe-sparse-operations*` is false, return `v`; + otherwise, throw an `ExceptionInfo` with this `message` and the map `m`." + [v message m] + (if-not + v + (if + *safe-sparse-operations* + (throw (ex-info message m)) + v) + v)) + (defn sparse-array? "`true` if `x` is a sparse array conforming to the conventions established by this library, else `false`." @@ -31,14 +52,27 @@ (:content x)))))) ([x & axes] (and - (map? x) - (number? (:dimensions x)) - (pos? (:dimensions x)) - (keyword? (:coord x)) - (= (:coord x) (first axes)) + (safe-test-or-throw + (map? x) + "Array must be a map" {:array x}) + (safe-test-or-throw + (and (integer? (:dimensions x)) (pos? (:dimensions x))) + (str "The value of `:dimensions` must be a positive integer, not " (:dimensions x)) + {:array x}) + (safe-test-or-throw + (keyword? (:coord x)) + (str "The value of `:coord` must be a keyword, not " (:coord x)) + {:array x}) + (safe-test-or-throw + (= (:coord x) (first axes)) + (str "The value of `:coord` must be " (first axes) ", not " (:coord x)) + {:array x}) (if (empty? (rest axes)) - (= (:content x) :data) + (safe-test-or-throw + (= (:content x) :data) + "If there are no further axes the value of `:content` must be `:data`" + {:array x}) (and (= (:content x) (rest axes)) (every? @@ -50,7 +84,7 @@ `coordinates`. Returns `nil` if any coordinate is invalid." [array value & coordinates] (cond - (and *safe-sparse-operations* (sparse-array? array)) + (and *safe-sparse-operations* (not (sparse-array? array))) (throw (ex-info "Sparse array expected" {:array array})) (every? #(and (integer? %) (or (zero? %) (pos? %))) @@ -72,16 +106,31 @@ (throw (ex-info "Coordinates must be zero or positive integers" - {:coordinates coordinates - :invalid (remove integer? coordinates)})))) + {:array array + :coordinates coordinates + :invalid (remove #(and (pos? %) (integer? %)) coordinates)})))) (defn get "Return the value in this sparse `array` at these `coordinates`." ;; TODO: I am CERTAIN there is a more elegant solution to this. [array & coordinates] - (if + (cond + *safe-sparse-operations* + (cond + (not (sparse-array? array)) + (throw (ex-info "Sparse array expected" {:array array})) + (not (every? + #(and (integer? %) (or (zero? %) (pos? %))) + coordinates)) + (throw + (ex-info + "Coordinates must be zero or positive integers" + {:array array + :coordinates coordinates + :invalid (remove #(and (pos? %) (integer? %)) coordinates)}))) (= :data (:content array)) (array (first coordinates)) + :else (apply get (cons (array (first coordinates)) (rest coordinates))))) (defn merge-sparse-arrays @@ -98,7 +147,7 @@ nil (= :data (:content a1)) (merge a1 a2) - :else + (or (unsafe-sparse-operations?) (and (sparse-array? a1) (sparse-array? a2))) (reduce merge a2 diff --git a/test/sparse_array/core_test.clj b/test/sparse_array/core_test.clj index b7d5707..5ec9c85 100644 --- a/test/sparse_array/core_test.clj +++ b/test/sparse_array/core_test.clj @@ -50,6 +50,49 @@ "Can't have data in a non-data layer") )) +(deftest testing-safe + (testing "Checking that correct exceptions are thrown when `*safe-sparse-operations*` is true" + (binding [*safe-sparse-operations* true] + (is + (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Array must be a map" + (sparse-array? []))) + (is + (thrown-with-msg? + clojure.lang.ExceptionInfo + #"The value of `:dimensions` must be a positive integer, not .*" + (sparse-array? + (dissoc (make-sparse-array :x :y :z) :dimensions))) + "All mandatory keywords must be present: dimensions") + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"The value of `:coord` must be a keyword, not .*" + (sparse-array? + (dissoc (make-sparse-array :x :y :z) :coord)))) + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"If there are no further axes the value of `:content` must be `:data`" + (sparse-array? + (dissoc (make-sparse-array :x :y :z) :content))) + "All mandatory keywords must be present: content") + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Array must be a map" + (sparse-array? {:dimensions 2, + :coord :x, + :content '(:y), + 3 {:dimensions 1, + :coord :y, + :content :data, + 4 "hello"}, + 4 {:dimensions 1, + :coord :y, + :content :data, + 3 "goodbye"} + 5 :foo})) + "Can't have data in a non-data layer")))) + (deftest put-and-get (testing "get" (let [expected "hello" From bc1af9cdb0f81f75e5bd0d0d6c1f69594dd8d43e Mon Sep 17 00:00:00 2001 From: Simon Brooke Date: Wed, 26 Jun 2019 12:08:30 +0100 Subject: [PATCH 4/5] lein-release plugin: preparing 0.1.0 release --- README.md | 23 ++++++++- project.clj | 2 +- src/sparse_array/core.clj | 92 +++++++++++++++++++++++---------- test/sparse_array/core_test.clj | 47 +++++++++++++++-- 4 files changed, 132 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index d7e0b74..a6d6be5 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,8 @@ A Clojure library designed to manipulate sparse *arrays* - multi-dimensional spa Arbitrary numbers of dimensions are supported, up to limits imposed by the JVM stack. +[![Clojars Project](https://img.shields.io/clojars/v/sparse-array.svg)](https://clojars.org/sparse-array) + ## Conventions: ### Sparse arrays @@ -32,7 +34,26 @@ Thus an array with a single value 'hello' at coordinates x = 3, y = 4, z = 5 wou } ``` -At the present stage of development, where the expectations of an operation are violated, `nil` is returned and no exception is thrown. However, it's probable that later there will be at least the option of thowing specific exceptions, as otherwise debugging could be tricky. +### Errors and error-reporting + +A dynamic variable, `*safe-sparse-operations*`, is provided to handle behaviour in error conditions. If this is `false`, bad data will generally not cause an exception to be thrown, and corrupt structures may be returned, thus: + +```clojure +(put (make-sparse-array :x :y :z) "hello" 3) ;; insufficient coordinates specified + +=> {:dimensions 3, :coord :x, :content (:y :z), 3 {:dimensions 2, :coord :y, :content (:z), nil {:dimensions 1, :coord :z, :content :data, nil nil}}} +``` + +However, if `*safe-sparse-operations*` is bound to `true`, exceptions will be thrown instead: + +```clojure +(binding [*safe-sparse-operations* true] + (put (make-sparse-array :x :y :z) "hello" 3)) + +ExceptionInfo Expected 3 coordinates; found 1 clojure.core/ex-info (core.clj:4617) +``` + +Sanity checking data is potentially expensive; for this reason `*safe-sparse-operations*` defaults to `false`, but you make wish to bind it to `true` especially while debugging. ### Dense arrays diff --git a/project.clj b/project.clj index fb40d19..8130d19 100644 --- a/project.clj +++ b/project.clj @@ -1,4 +1,4 @@ -(defproject sparse-array "0.1.0-SNAPSHOT" +(defproject sparse-array "0.1.0" :description "A Clojure library designed to manipulate sparse *arrays* - multi-dimensional spaces accessed by indices, but containing arbitrary values rather than just numbers. For sparse spaces which contain numbers only, you're better to use a *sparse matrix* library, for example [clojure.core.matrix](https://mikera.github.io/core.matrix/)." :url "http://example.com/FIXME" :license {:name "Eclipse Public License" diff --git a/src/sparse_array/core.clj b/src/sparse_array/core.clj index ca98565..c984580 100644 --- a/src/sparse_array/core.clj +++ b/src/sparse_array/core.clj @@ -1,12 +1,14 @@ (ns sparse-array.core) +(declare put get) + (def ^:dynamic *safe-sparse-operations* "Whether spase array operations should be conducted safely, with careful checking of data conventions and exceptions thrown if expectations are not met. Normally `false`." false) -(defmacro unsafe-sparse-operations? +(defn- unsafe-sparse-operations? "returns `true` if `*safe-sparse-operations*` is `false`, and vice versa." [] (not (true? *safe-sparse-operations*))) @@ -79,13 +81,9 @@ sparse-array? (map #(x %) (filter integer? (keys x))))))))) -(defn put - "Return a sparse array like this `array` but with this `value` at these - `coordinates`. Returns `nil` if any coordinate is invalid." - [array value & coordinates] +(defn- unsafe-put + [array value coordinates] (cond - (and *safe-sparse-operations* (not (sparse-array? array))) - (throw (ex-info "Sparse array expected" {:array array})) (every? #(and (integer? %) (or (zero? %) (pos? %))) coordinates) @@ -101,37 +99,77 @@ (or (array (first coordinates)) (apply make-sparse-array (:content array))) - (cons value (rest coordinates)))))) - *safe-sparse-operations* + (cons value (rest coordinates)))))))) + +(defn put + "Return a sparse array like this `array` but with this `value` at these + `coordinates`. Returns `nil` if any coordinate is invalid." + [array value & coordinates] + (cond + (nil? value) + nil + (unsafe-sparse-operations?) + (unsafe-put array value coordinates) + (not (sparse-array? array)) + (throw (ex-info "Sparse array expected" {:array array})) + (not= (:dimensions array) (count coordinates)) + (throw + (ex-info + (str "Expected " (:dimensions array) " coordinates; found " (count coordinates)) + {:array array + :coordinates coordinates})) + (not + (every? + #(and (integer? %) (or (zero? %) (pos? %))) + coordinates)) (throw (ex-info "Coordinates must be zero or positive integers" {:array array :coordinates coordinates - :invalid (remove #(and (pos? %) (integer? %)) coordinates)})))) + :invalid (remove #(and (pos? %) (integer? %)) coordinates)})) + :else + (unsafe-put array value coordinates) + value + *safe-sparse-operations*)) + +(defn- unsafe-get + ;; TODO: I am CERTAIN there is a more elegant solution to this. + [array coordinates] + (let [v (array (first coordinates))] + (cond + (= :data (:content array)) + v + (nil? v) + nil + :else + (apply get (cons v (rest coordinates)))))) (defn get "Return the value in this sparse `array` at these `coordinates`." - ;; TODO: I am CERTAIN there is a more elegant solution to this. [array & coordinates] (cond - *safe-sparse-operations* - (cond - (not (sparse-array? array)) - (throw (ex-info "Sparse array expected" {:array array})) - (not (every? - #(and (integer? %) (or (zero? %) (pos? %))) - coordinates)) - (throw - (ex-info - "Coordinates must be zero or positive integers" - {:array array - :coordinates coordinates - :invalid (remove #(and (pos? %) (integer? %)) coordinates)}))) - (= :data (:content array)) - (array (first coordinates)) + (unsafe-sparse-operations?) + (unsafe-get array coordinates) + (not (sparse-array? array)) + (throw (ex-info "Sparse array expected" {:array array})) + (not (every? + #(and (integer? %) (or (zero? %) (pos? %))) + coordinates)) + (throw + (ex-info + "Coordinates must be zero or positive integers" + {:array array + :coordinates coordinates + :invalid (remove #(and (pos? %) (integer? %)) coordinates)})) + (not (= (:dimensions array) (count coordinates))) + (throw + (ex-info + (str "Expected " (:dimensions array) " coordinates; found " (count coordinates)) + {:array array + :coordinates coordinates})) :else - (apply get (cons (array (first coordinates)) (rest coordinates))))) + (unsafe-get array coordinates))) (defn merge-sparse-arrays "Return a sparse array taking values from sparse arrays `a1` and `a2`, diff --git a/test/sparse_array/core_test.clj b/test/sparse_array/core_test.clj index 5ec9c85..b06f2ad 100644 --- a/test/sparse_array/core_test.clj +++ b/test/sparse_array/core_test.clj @@ -101,6 +101,13 @@ :content '(:y) 3 {:dimensions 1 :coord :y :content :data 4 "hello"}} actual (get array 3 4)] + (is (= actual expected))) + (let [expected nil + array {:dimensions 2, + :coord :x, + :content '(:y) + 3 {:dimensions 1 :coord :y :content :data 4 "hello"}} + actual (get array 4 3)] (is (= actual expected)))) (testing "put" (let [expected "hello" @@ -113,12 +120,24 @@ (let [expected "hello" actual (get (put (make-sparse-array :x) expected 3) 3)] - (is (= actual expected)))) + (is (= actual expected))) + (binding [*safe-sparse-operations* true] + ;; enabling error handling shouldn't make any difference + (let + [expected "hello" + actual (get (put (make-sparse-array :x) expected 3) 3)] + (is (= actual expected))))) (testing "round trip, two dimensions" (let [expected "hello" actual (get (put (make-sparse-array :x :y) expected 3 4) 3 4)] - (is (= actual expected)))) + (is (= actual expected))) + (binding [*safe-sparse-operations* true] + ;; enabling error handling shouldn't make any difference + (let + [expected "hello" + actual (get (put (make-sparse-array :x :y) expected 3 4) 3 4)] + (is (= actual expected))))) (testing "round trip, three dimensions" (let [expected "hello" @@ -128,7 +147,29 @@ (let [expected "hello" actual (get (put (make-sparse-array :p :q :r :s) expected 3 4 5 6) 3 4 5 6)] - (is (= actual expected))))) + (is (= actual expected)))) + (testing "Error handling, number of dimensions" + (binding [*safe-sparse-operations* true] + (is + (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Expected 3 coordinates; found 2" + (put (make-sparse-array :x :y :z) "hello" 3 4))) + (is + (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Expected 3 coordinates; found 4" + (put (make-sparse-array :x :y :z) "hello" 3 4 5 6))) + (is + (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Expected 3 coordinates; found 2" + (get (make-sparse-array :x :y :z) 3 4))) + (is + (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Expected 3 coordinates; found 4" + (get (make-sparse-array :x :y :z) 3 4 5 6)))))) (deftest merge-test (testing "merge, one dimension" From 958413d2e3f5b5951e538b260a0126281d0f7cc2 Mon Sep 17 00:00:00 2001 From: Simon Brooke Date: Wed, 26 Jun 2019 13:26:45 +0100 Subject: [PATCH 5/5] 0.1.0 release build --- project.clj | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/project.clj b/project.clj index 8130d19..10fba9b 100644 --- a/project.clj +++ b/project.clj @@ -5,6 +5,10 @@ :url "http://www.eclipse.org/legal/epl-v10.html"} :dependencies [[org.clojure/clojure "1.8.0"]] + :plugins [[lein-codox "0.10.4"] + [lein-release "1.0.5"]] + + ;; `lein release` doesn't play nice with `git flow release`. Run `lein release` in the ;; `develop` branch, then merge the release tag into the `master` branch.