core.logic

Swapping noms turns maps (and other collections) into seqs

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Approval:
    Accepted

Description

When I unify two binders, the swapping procedure turns all the maps inside the body into seqs of key/value pairs, which do not unify with maps, leading to the following trickiness:

(require '[clojure.core.logic :as l])
(require '[clojure.core.logic.nominal :as n])
(l/run* [q]
  (l/fresh [body]
    (n/fresh [a b]
      (l/== (n/tie a {:k a}) (n/tie b body))
      (l/== {:k q} body))))
;=> ()
; Expected (a_0)

For my purposes, fixing this by adding two new implementations of INomSwap for vectors and maps works well.

Activity

Hide
Jiří Maršík added a comment -

Oops, sorry for the malformatted code snippet. Here it is inside a code tag.

(require '[clojure.core.logic :as l])
(require '[clojure.core.logic.nominal :as n])
(l/run* [q]
        (l/fresh [body]
                 (n/fresh [a b]
                          (l/== (n/tie a {:k a}) (n/tie b body))
                          (l/== {:k q} body))))
;=> ()
; Expected (a_0)
Show
Jiří Maršík added a comment - Oops, sorry for the malformatted code snippet. Here it is inside a code tag.
(require '[clojure.core.logic :as l])
(require '[clojure.core.logic.nominal :as n])
(l/run* [q]
        (l/fresh [body]
                 (n/fresh [a b]
                          (l/== (n/tie a {:k a}) (n/tie b body))
                          (l/== {:k q} body))))
;=> ()
; Expected (a_0)
David Nolen made changes -
Field Original Value New Value
Description When I unify two binders, the swapping procedure turns all the maps inside the body into seqs of key/value pairs, which do not unify with maps, leading to the following trickiness:

(require '[clojure.core.logic :as l])
(require '[clojure.core.logic.nominal :as n])
(l/run* [q]
        (l/fresh [body]
                 (n/fresh [a b]
                          (l/== (n/tie a {:k a}) (n/tie b body))
                                (l/== {:k q} body))))
;=> ()
; Expected (a_0)

For my purposes, fixing this by adding two new implementations of INomSwap for vectors and maps works well.
When I unify two binders, the swapping procedure turns all the maps inside the body into seqs of key/value pairs, which do not unify with maps, leading to the following trickiness:

{code}
(require '[clojure.core.logic :as l])
(require '[clojure.core.logic.nominal :as n])
(l/run* [q]
  (l/fresh [body]
    (n/fresh [a b]
      (l/== (n/tie a {:k a}) (n/tie b body))
      (l/== {:k q} body))))
;=> ()
; Expected (a_0)
{code}

For my purposes, fixing this by adding two new implementations of INomSwap for vectors and maps works well.
David Nolen made changes -
Assignee David Nolen [ dnolen ] Nada Amin [ namin ]
Hide
David Nolen added a comment -

This seems like a easy one to fix, perhaps Nada can see quicker than I can. If not I can take a look.

Also your patch may very well solve the issue best, feel free to attach it to the ticket. We won't be able to apply it until you've submitted your Contributor Agreement - if you have a free moment please send it in. Thanks!

Show
David Nolen added a comment - This seems like a easy one to fix, perhaps Nada can see quicker than I can. If not I can take a look. Also your patch may very well solve the issue best, feel free to attach it to the ticket. We won't be able to apply it until you've submitted your Contributor Agreement - if you have a free moment please send it in. Thanks!
Hide
Jiří Maršík added a comment -

The patch that I use to fix the issue. I haven't signed the CA, but I would definitely like to. Might take a while to arrive though, since I'm in Europe and I'm lazy.

Show
Jiří Maršík added a comment - The patch that I use to fix the issue. I haven't signed the CA, but I would definitely like to. Might take a while to arrive though, since I'm in Europe and I'm lazy.
Jiří Maršík made changes -
Attachment INomSwap-vectors-maps.patch [ 11936 ]
Hide
Nada Amin added a comment -

The patch LGTM. I would also add a test from the ticket example.

I guess we have to wait until the CA arrives, now.

Show
Nada Amin added a comment - The patch LGTM. I would also add a test from the ticket example. I guess we have to wait until the CA arrives, now.
Hide
Jiří Maršík added a comment -

I have also added the test case, the new patch INomSwap-vectors-maps-with-test.patch contains both the fix and the test case. I have just signed the CA and I plan to post it today.

Show
Jiří Maršík added a comment - I have also added the test case, the new patch INomSwap-vectors-maps-with-test.patch contains both the fix and the test case. I have just signed the CA and I plan to post it today.
Jiří Maršík made changes -
Hide
Jiří Maršík added a comment -

OK, the CA has arrived and been processed.

http://clojure.org/contributing

  • Jiri Marsik (jirkamarsik)
Show
Jiří Maršík added a comment - OK, the CA has arrived and been processed. http://clojure.org/contributing
  • Jiri Marsik (jirkamarsik)
Nada Amin made changes -
Resolution Completed [ 1 ]
Approval Accepted [ 10005 ]
Status Open [ 1 ] Resolved [ 5 ]
David Nolen made changes -
Status Resolved [ 5 ] Closed [ 6 ]

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: