Clojure

clojure.data/diff should cope with null and false values in maps

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: Release 1.3, Release 1.4
  • Fix Version/s: Release 1.5
  • Component/s: None
  • Labels:
  • Patch:
    Code and Test
  • Approval:
    Ok

Description

Current behaviour of clojure.data/diff:

=> (diff {:a false} {:a true})
(nil {:a true} nil)
=> (diff {:a false} {:a nil})
(nil nil nil)

Proposed behaviour:

=> (diff {:a false} {:a true})
({:a false} {:a true} nil)
=> (diff {:a false} {:a nil})
({:a false} {:a nil} nil)

Activity

Descending order - Click to sort in ascending order
Hide
Permalink
Aaron Bedra added a comment -

Patch applies cleanly against 4004d267e124f12b65b0d7fb6522f32a75e3c4fb. Submitter is a confirmed CA signer.

Show
Aaron Bedra added a comment - Patch applies cleanly against 4004d267e124f12b65b0d7fb6522f32a75e3c4fb. Submitter is a confirmed CA signer.
Hide
Permalink
Stuart Halloway added a comment -

Yeah, this should be fixed.

Show
Stuart Halloway added a comment - Yeah, this should be fixed.
Hide
Permalink
Philip Aston added a comment -

I now have a CA in place.

Show
Philip Aston added a comment - I now have a CA in place.
Hide
Permalink
Philip Aston added a comment -

CA snail-mailed yesterday, I guess it will take a week to arrive.

Show
Philip Aston added a comment - CA snail-mailed yesterday, I guess it will take a week to arrive.
Hide
Permalink
Philip Aston added a comment -

Thanks Andy. Patch attached:

0001-clojure.data-diff-cope-with-falsey-values-in-maps.patch

I'll send in a CA.

Show
Philip Aston added a comment - Thanks Andy. Patch attached: 0001-clojure.data-diff-cope-with-falsey-values-in-maps.patch I'll send in a CA.
Hide
Permalink
Andy Fingerhut added a comment -

Philip, thanks for writing that patch. Could you please attach it as a file to this ticket, instead of copying and pasting it into a comment? Instructions are under the heading "Adding patches" on this page:

http://dev.clojure.org/display/design/JIRA+workflow

Rich Hickey's policy is only to include code in Clojure that is written by those who have signed a contributor agreement. You can find out more at http://clojure.org/contributing

Show
Andy Fingerhut added a comment - Philip, thanks for writing that patch. Could you please attach it as a file to this ticket, instead of copying and pasting it into a comment? Instructions are under the heading "Adding patches" on this page: http://dev.clojure.org/display/design/JIRA+workflow Rich Hickey's policy is only to include code in Clojure that is written by those who have signed a contributor agreement. You can find out more at http://clojure.org/contributing
Hide
Permalink
Philip Aston added a comment - - edited 
 
From e03a8060214d122ea2ebadf9e8a368f7f593d9f4 Mon Sep 17 00:00:00 2001
From: Philip Aston <philipa@mail.com>
Date: Sun, 10 Jun 2012 13:11:36 +0100
Subject: [PATCH] clojure.data/diff: cope with falsey values in maps

---
 src/clj/clojure/data.clj           |   18 +++++++++++++++++-
 test/clojure/test_clojure/data.clj |    3 ++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/clj/clojure/data.clj b/src/clj/clojure/data.clj
index 6e8dbcf..345b234 100644
--- a/src/clj/clojure/data.clj
+++ b/src/clj/clojure/data.clj
@@ -30,6 +30,22 @@
      (vec (repeat (apply max (keys m))  nil))
      m)))
 
+(defn- diff-associative-key
+  "Diff associative things a and b, comparing only the key k."
+  [a b k]
+  (let [va (get a k)
+        vb (get b k)
+        [a* b* ab] (diff va vb)
+        in-a (contains? a k)
+        in-b (contains? b k)
+        same (and in-a in-b
+                  (or (not (nil? ab))
+                      (and (nil? va) (nil? vb))))]
+    [(when (and in-a (or (not (nil? a*)) (not same))) {k a*})
+     (when (and in-b (or (not (nil? b*)) (not same))) {k b*})
+     (when same {k ab})
+     ]))
+
 (defn- diff-associative
   "Diff associative things a and b, comparing only keys in ks."
   [a b ks]
@@ -38,7 +54,7 @@
      (doall (map merge diff1 diff2)))
    [nil nil nil]
    (map
-    (fn [k] (map #(when % {k %}) (diff (get a k) (get b k))))
+    (partial diff-associative-key a b)
     ks)))
 
 (defn- diff-sequential
diff --git a/test/clojure/test_clojure/data.clj b/test/clojure/test_clojure/data.clj
index 9bab766..5a241e0 100644
--- a/test/clojure/test_clojure/data.clj
+++ b/test/clojure/test_clojure/data.clj
@@ -27,5 +27,6 @@
        [#{1} #{3} #{2}] (HashSet. [1 2]) (HashSet. [2 3])
        [nil nil [1 2]] [1 2] (into-array [1 2])
        [nil nil [1 2]] (into-array [1 2]) [1 2]
-       [{:a {:c [1]}} {:a {:c [0]}} {:a {:c [nil 2] :b 1}}] {:a {:b 1 :c [1 2]}} {:a {:b 1 :c [0 2]}}))
+       [{:a {:c [1]}} {:a {:c [0]}} {:a {:c [nil 2] :b 1}}] {:a {:b 1 :c [1 2]}} {:a {:b 1 :c [0 2]}}
+       [{:a nil} {:a false} {:b nil :c false}] {:a nil :b nil :c false} {:a false :b nil :c false}))
 
-- 
1.7.9.5
Show
Philip Aston added a comment - - edited 
 
From e03a8060214d122ea2ebadf9e8a368f7f593d9f4 Mon Sep 17 00:00:00 2001
From: Philip Aston <philipa@mail.com>
Date: Sun, 10 Jun 2012 13:11:36 +0100
Subject: [PATCH] clojure.data/diff: cope with falsey values in maps

---
 src/clj/clojure/data.clj           |   18 +++++++++++++++++-
 test/clojure/test_clojure/data.clj |    3 ++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/clj/clojure/data.clj b/src/clj/clojure/data.clj
index 6e8dbcf..345b234 100644
--- a/src/clj/clojure/data.clj
+++ b/src/clj/clojure/data.clj
@@ -30,6 +30,22 @@
      (vec (repeat (apply max (keys m))  nil))
      m)))
 
+(defn- diff-associative-key
+  "Diff associative things a and b, comparing only the key k."
+  [a b k]
+  (let [va (get a k)
+        vb (get b k)
+        [a* b* ab] (diff va vb)
+        in-a (contains? a k)
+        in-b (contains? b k)
+        same (and in-a in-b
+                  (or (not (nil? ab))
+                      (and (nil? va) (nil? vb))))]
+    [(when (and in-a (or (not (nil? a*)) (not same))) {k a*})
+     (when (and in-b (or (not (nil? b*)) (not same))) {k b*})
+     (when same {k ab})
+     ]))
+
 (defn- diff-associative
   "Diff associative things a and b, comparing only keys in ks."
   [a b ks]
@@ -38,7 +54,7 @@
      (doall (map merge diff1 diff2)))
    [nil nil nil]
    (map
-    (fn [k] (map #(when % {k %}) (diff (get a k) (get b k))))
+    (partial diff-associative-key a b)
     ks)))
 
 (defn- diff-sequential
diff --git a/test/clojure/test_clojure/data.clj b/test/clojure/test_clojure/data.clj
index 9bab766..5a241e0 100644
--- a/test/clojure/test_clojure/data.clj
+++ b/test/clojure/test_clojure/data.clj
@@ -27,5 +27,6 @@
        [#{1} #{3} #{2}] (HashSet. [1 2]) (HashSet. [2 3])
        [nil nil [1 2]] [1 2] (into-array [1 2])
        [nil nil [1 2]] (into-array [1 2]) [1 2]
-       [{:a {:c [1]}} {:a {:c [0]}} {:a {:c [nil 2] :b 1}}] {:a {:b 1 :c [1 2]}} {:a {:b 1 :c [0 2]}}))
+       [{:a {:c [1]}} {:a {:c [0]}} {:a {:c [nil 2] :b 1}}] {:a {:b 1 :c [1 2]}} {:a {:b 1 :c [0 2]}}
+       [{:a nil} {:a false} {:b nil :c false}] {:a nil :b nil :c false} {:a false :b nil :c false}))
 
-- 
1.7.9.5

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4#649-r158309) | Report a problem
Powered by a free Atlassian JIRA open source license for Clojure. Try JIRA - bug tracking software for your team.