Hey:
Here's a destructive/non-consing version of DELETE-FROM-PLIST. I've tested with (I think) all corner cases from the REPL, but I ought to write tests proper.
The function is here http://tinyurl.com/adqkssu ;or the following huge link, in case the last one is invalidated https://bitbucket.org/tarballs_are_good/lisp-random/src/3db634111d35e788c6ea... .
For simplicity or ease of review from an email client, I've pasted the function at the end of this email.
Additionally, this function would make it pretty easy to write DELETE-FROM-PLIST-IF{-NOT}, since the function to determine if a key is bad is factored out. If one did write this function, then it would be easy to define DELETE-FROM-PLIST in terms of it.
Let me know if there are any changes that should be made.
Cheers,
Robert Smith
;;;; from delete-from-plist.lisp
(defun delete-from-plist (plist &rest keys) "Delete all keys and pairs indicated by KEYS from the plist PLIST." (labels ((assert-proper-plist (x) (assert x () "Expected a proper plist, got ~S" plist)) (bad-key-p (key) (member key keys :test #'eq)) (find-first () "Find the first cons in PLIST to keep." (loop :for the-cons :on plist :by #'cddr :unless (prog1 (bad-key-p (car the-cons)) (assert-proper-plist (cdr the-cons))) :do (return the-cons) :finally (return nil)))) (declare (inline assert-proper-plist bad-key-p find-first)) ;; Find the first good key and delete any bad key-value pairs ;; between it and the start. (let ((first (find-first))) (unless (eq first plist) (setf (cddr plist) first))
;; At this point, we know FIRST points to the first key ;; which exists, or NIL. (loop :with last-good := first ; Keep the last good key :for the-cons :on (cddr first) :by #'cddr :do (progn (assert-proper-plist (cdr the-cons)) (if (bad-key-p (car the-cons)) (setf (cddr last-good) (cddr the-cons)) (setf last-good the-cons))) :finally (return first)))))
Attached is a proper patch.
-Robert
On Sat, Feb 23, 2013 at 1:09 AM, Robert Smith quad@symbo1ics.com wrote:
Hey:
Here's a destructive/non-consing version of DELETE-FROM-PLIST. I've tested with (I think) all corner cases from the REPL, but I ought to write tests proper.
The function is here http://tinyurl.com/adqkssu ;or the following huge link, in case the last one is invalidated https://bitbucket.org/tarballs_are_good/lisp-random/src/3db634111d35e788c6ea... .
For simplicity or ease of review from an email client, I've pasted the function at the end of this email.
Additionally, this function would make it pretty easy to write DELETE-FROM-PLIST-IF{-NOT}, since the function to determine if a key is bad is factored out. If one did write this function, then it would be easy to define DELETE-FROM-PLIST in terms of it.
Let me know if there are any changes that should be made.
Cheers,
Robert Smith
;;;; from delete-from-plist.lisp
(defun delete-from-plist (plist &rest keys) "Delete all keys and pairs indicated by KEYS from the plist PLIST." (labels ((assert-proper-plist (x) (assert x () "Expected a proper plist, got ~S" plist)) (bad-key-p (key) (member key keys :test #'eq)) (find-first () "Find the first cons in PLIST to keep." (loop :for the-cons :on plist :by #'cddr :unless (prog1 (bad-key-p (car the-cons)) (assert-proper-plist (cdr the-cons))) :do (return the-cons) :finally (return nil)))) (declare (inline assert-proper-plist bad-key-p find-first)) ;; Find the first good key and delete any bad key-value pairs ;; between it and the start. (let ((first (find-first))) (unless (eq first plist) (setf (cddr plist) first))
;; At this point, we know FIRST points to the first key ;; which exists, or NIL. (loop :with last-good := first ; Keep the last good key :for the-cons :on (cddr first) :by #'cddr :do (progn (assert-proper-plist (cdr the-cons)) (if (bad-key-p (car the-cons)) (setf (cddr last-good) (cddr the-cons)) (setf last-good the-cons))) :finally (return first)))))
Robert Smith quad@symbo1ics.com writes:
Hey:
Here's a destructive/non-consing version of DELETE-FROM-PLIST. I've tested with (I think) all corner cases from the REPL, but I ought to write tests proper.
The function is here http://tinyurl.com/adqkssu ;or the following huge link, in case the last one is invalidated https://bitbucket.org/tarballs_are_good/lisp-random/src/3db634111d35e788c6ea... .
For simplicity or ease of review from an email client, I've pasted the function at the end of this email.
Additionally, this function would make it pretty easy to write DELETE-FROM-PLIST-IF{-NOT}, since the function to determine if a key is bad is factored out. If one did write this function, then it would be easy to define DELETE-FROM-PLIST in terms of it.
Let me know if there are any changes that should be made.
Cheers,
Robert Smith
;;;; from delete-from-plist.lisp
(defun delete-from-plist (plist &rest keys) "Delete all keys and pairs indicated by KEYS from the plist PLIST." (labels ((assert-proper-plist (x) (assert x () "Expected a proper plist, got ~S" plist)) (bad-key-p (key) (member key keys :test #'eq)) (find-first () "Find the first cons in PLIST to keep." (loop :for the-cons :on plist :by #'cddr :unless (prog1 (bad-key-p (car the-cons)) (assert-proper-plist (cdr the-cons))) :do (return the-cons) :finally (return nil)))) (declare (inline assert-proper-plist bad-key-p find-first)) ;; Find the first good key and delete any bad key-value pairs ;; between it and the start. (let ((first (find-first))) (unless (eq first plist) (setf (cddr plist) first))
;; At this point, we know FIRST points to the first key ;; which exists, or NIL. (loop :with last-good := first ; Keep the last good key :for the-cons :on (cddr first) :by #'cddr :do (progn (assert-proper-plist (cdr the-cons)) (if (bad-key-p (car the-cons)) (setf (cddr last-good) (cddr the-cons)) (setf last-good the-cons))) :finally (return first)))))
Alexandria is not using :keyword style for LOOP. DO in loop has an implicit progn, no need for an explicit one. unless (prog1 ... (assert)) can be better expressed as do (assert) unless ... return cons and finally (return nil) is not needed.
And I don't quite understand the purpose of (unless (eq first plist) (setf (cddr plist) first))
Good style remarks, I suppose, which I can change.
On Sat, Feb 23, 2013 at 8:08 AM, Stas Boukarev stassats@gmail.com wrote:
And I don't quite understand the purpose of (unless (eq first plist) (setf (cddr plist) first))
The point of that was for more DWIMness.
Without it, we have:
CL-USER> (let ((x (list :a 1 :b 2 :c 3 :d 4))) (print (delete-from-plist x :a :b :c)) x)
(:D 4) (:A 1 :B 2 :C 3 :D 4)
And with it, we have:
CL-USER> (let ((x (list :a 1 :b 2 :c 3 :d 4))) (print (delete-from-plist x :a :b :c)) x)
(:D 4) (:A 1 :D 4)
If your typical mode of operation is to (setf x (delete-from...)), then of course either are fine. If your goal is to use DELETE-FROM-PLIST for its side effects, then I think the latter is more useful. In fact, for really simple and efficient imperative maintenance of a plist, just prepend (nil nil), and use the function purely for mutation.
Cheers,
Robert
Robert Smith quad@symbo1ics.com writes:
Good style remarks, I suppose, which I can change.
On Sat, Feb 23, 2013 at 8:08 AM, Stas Boukarev stassats@gmail.com wrote:
And I don't quite understand the purpose of (unless (eq first plist) (setf (cddr plist) first))
The point of that was for more DWIMness.
Without it, we have:
CL-USER> (let ((x (list :a 1 :b 2 :c 3 :d 4))) (print (delete-from-plist x :a :b :c)) x)
(:D 4) (:A 1 :B 2 :C 3 :D 4)
And with it, we have:
CL-USER> (let ((x (list :a 1 :b 2 :c 3 :d 4))) (print (delete-from-plist x :a :b :c)) x)
(:D 4) (:A 1 :D 4)
If your typical mode of operation is to (setf x (delete-from...)), then of course either are fine. If your goal is to use DELETE-FROM-PLIST for its side effects, then I think the latter is more useful. In fact, for really simple and efficient imperative maintenance of a plist, just prepend (nil nil), and use the function purely for mutation.
I don't see how (:A 1 :D 4) is useful, it's less wrong, but it's still wrong.
How is (:A 1 :D 4) wrong? There is no way to get it to just (:D 4) via mutation when passing to a function. If we want to have just (:D 4), we will need to either pass in
(nil nil <plist>)
which will give
(nil nil :D 4)
or we will need to encapsulate or box the value before going in, by using something like https://bitbucket.org/tarballs_are_good/cl-ref .
All in all, it still returns (:D 4), but it just modifies as much of the list structure as it can.
-Robert
On Sat, Feb 23, 2013 at 11:53 AM, Stas Boukarev stassats@gmail.com wrote:
Robert Smith quad@symbo1ics.com writes:
Good style remarks, I suppose, which I can change.
On Sat, Feb 23, 2013 at 8:08 AM, Stas Boukarev stassats@gmail.com wrote:
And I don't quite understand the purpose of (unless (eq first plist) (setf (cddr plist) first))
The point of that was for more DWIMness.
Without it, we have:
CL-USER> (let ((x (list :a 1 :b 2 :c 3 :d 4))) (print (delete-from-plist x :a :b :c)) x)
(:D 4) (:A 1 :B 2 :C 3 :D 4)
And with it, we have:
CL-USER> (let ((x (list :a 1 :b 2 :c 3 :d 4))) (print (delete-from-plist x :a :b :c)) x)
(:D 4) (:A 1 :D 4)
If your typical mode of operation is to (setf x (delete-from...)), then of course either are fine. If your goal is to use DELETE-FROM-PLIST for its side effects, then I think the latter is more useful. In fact, for really simple and efficient imperative maintenance of a plist, just prepend (nil nil), and use the function purely for mutation.
I don't see how (:A 1 :D 4) is useful, it's less wrong, but it's still wrong.
-- With best regards, Stas.
Robert Smith quad@symbo1ics.com writes:
How is (:A 1 :D 4) wrong? There is no way to get it to just (:D 4) via mutation when passing to a function. If we want to have just (:D 4), we will need to either pass in
It's wrong because it's completely useless, why would anyone use delete-from-plist without using the value returned by it, if the original list it modifies has the wrong result? Having to prepend two NILs is just bogus.
Although, that's not true that there's no way to have (:D 4), but the problem just shifts to when it deletes everything and it's a NIL:
(unless (eq first plist) (psetf (car plist) (car first) (cdr plist) (cdr first)))
But that part shouldn't be in alexandria (or any sane library, for that matter) either way, because it encourages erroneous usage, seemingly doing the right thing, but breaks when it comes to returning NIL.
And there's alexandria:delete-from-plistf for people who are afraid of an extra SETF.
On Sat, Feb 23, 2013 at 4:19 PM, Stas Boukarev stassats@gmail.com wrote:
It's wrong because it's completely useless, why would anyone use delete-from-plist without using the value returned by it, if the original list it modifies has the wrong result? Having to prepend two NILs is just bogus.
1. Because some people like or prefer to modify data structures (especially when you have elaborate data structures), and not bindings.
2. Having to prepend two NILs is fine I think. Yes it is hacky, but I don't see any inherent issue with it. It just establishes a (few) conses that act as the "head" or "entry point" to the list.
Although, that's not true that there's no way to have (:D 4), but the problem just shifts to when it deletes everything and it's a NIL:
(unless (eq first plist) (psetf (car plist) (car first) (cdr plist) (cdr first)))
I suppose you're correct here. I was implicitly assuming that we would want to never modify the CAR, but that is a sort of useless assumption.
But that part shouldn't be in alexandria (or any sane library, for that matter) either way, because it encourages erroneous usage, seemingly doing the right thing, but breaks when it comes to returning NIL.
I don't think it's erroneous. We aren't conflating the ideas of modifying a data structure and modifying a binding. By not doing that extra mutation, we rely on the user to "finish the job" by re-setting their variable to the new value.
The only thing "weird" about it is that there are a lot of Lisp functions which don't "completely" mutate the data structure, and expect the user to modify the binding. As far as I can tell, then, the really only argument against such a thing is "that's not how other CL functions work, so for consistency's sake, we shouldn't either." Unless I'm missing some point.
And there's alexandria:delete-from-plistf for people who are afraid of an extra SETF.
In this, we are trading a purely functional (i.e., non-special/macro) solution for a macro solution. Doesn't that go against the grain of the prevalent ideology of Lisp?
Cheers,
Robert
Robert Smith quad@symbo1ics.com writes:
On Sat, Feb 23, 2013 at 4:19 PM, Stas Boukarev stassats@gmail.com wrote:
It's wrong because it's completely useless, why would anyone use delete-from-plist without using the value returned by it, if the original list it modifies has the wrong result? Having to prepend two NILs is just bogus.
- Because some people like or prefer to modify data structures
(especially when you have elaborate data structures), and not bindings.
Well, those people should learn that it's not possible in general. Particularly with lists, because of NIL.
- Having to prepend two NILs is fine I think. Yes it is hacky, but I
don't see any inherent issue with it. It just establishes a (few) conses that act as the "head" or "entry point" to the list.
What if the key you want to remove is NIL?
But that part shouldn't be in alexandria (or any sane library, for that matter) either way, because it encourages erroneous usage, seemingly doing the right thing, but breaks when it comes to returning NIL.
I don't think it's erroneous. We aren't conflating the ideas of modifying a data structure and modifying a binding. By not doing that extra mutation, we rely on the user to "finish the job" by re-setting their variable to the new value.
There's no other way to obtain the correct results otherwise. Having it to work in 99% is worse than having it not to work at all, because people might forget about the remaining 1% case more easily. So you end up with doing extra work which has no use. The goal is simplicity, not having to memorize in which cases it's alright and which it's not. Just use the return value and be merry.
And there's alexandria:delete-from-plistf for people who are afraid of an extra SETF.
In this, we are trading a purely functional (i.e., non-special/macro) solution for a macro solution. Doesn't that go against the grain of the prevalent ideology of Lisp?
There's no trading, delete-from-plistf is just a define-modify-macro for delete-from-plist.
Fair enough, I feel sufficiently convinced.
If only we could construct NIL from a cons cell, eh?
Thanks,
Robert
On Sat, Feb 23, 2013 at 5:23 PM, Stas Boukarev stassats@gmail.com wrote:
Robert Smith quad@symbo1ics.com writes:
On Sat, Feb 23, 2013 at 4:19 PM, Stas Boukarev stassats@gmail.com wrote:
It's wrong because it's completely useless, why would anyone use delete-from-plist without using the value returned by it, if the original list it modifies has the wrong result? Having to prepend two NILs is just bogus.
- Because some people like or prefer to modify data structures
(especially when you have elaborate data structures), and not bindings.
Well, those people should learn that it's not possible in general. Particularly with lists, because of NIL.
- Having to prepend two NILs is fine I think. Yes it is hacky, but I
don't see any inherent issue with it. It just establishes a (few) conses that act as the "head" or "entry point" to the list.
What if the key you want to remove is NIL?
But that part shouldn't be in alexandria (or any sane library, for that matter) either way, because it encourages erroneous usage, seemingly doing the right thing, but breaks when it comes to returning NIL.
I don't think it's erroneous. We aren't conflating the ideas of modifying a data structure and modifying a binding. By not doing that extra mutation, we rely on the user to "finish the job" by re-setting their variable to the new value.
There's no other way to obtain the correct results otherwise. Having it to work in 99% is worse than having it not to work at all, because people might forget about the remaining 1% case more easily. So you end up with doing extra work which has no use. The goal is simplicity, not having to memorize in which cases it's alright and which it's not. Just use the return value and be merry.
And there's alexandria:delete-from-plistf for people who are afraid of an extra SETF.
In this, we are trading a purely functional (i.e., non-special/macro) solution for a macro solution. Doesn't that go against the grain of the prevalent ideology of Lisp?
There's no trading, delete-from-plistf is just a define-modify-macro for delete-from-plist.
-- With best regards, Stas.
On Sat, Feb 23, 2013 at 4:09 AM, Robert Smith quad@symbo1ics.com wrote:
(defun delete-from-plist (plist &rest keys) "Delete all keys and pairs indicated by KEYS from the plist PLIST." (labels ((assert-proper-plist (x) (assert x () "Expected a proper plist, got ~S" plist)) (bad-key-p (key) (member key keys :test #'eq)) (find-first () "Find the first cons in PLIST to keep." (loop :for the-cons :on plist :by #'cddr :unless (prog1 (bad-key-p (car the-cons)) (assert-proper-plist (cdr the-cons))) :do (return the-cons) :finally (return nil)))) (declare (inline assert-proper-plist bad-key-p find-first)) ;; Find the first good key and delete any bad key-value pairs ;; between it and the start. (let ((first (find-first))) (unless (eq first plist) (setf (cddr plist) first))
;; At this point, we know FIRST points to the first key ;; which exists, or NIL. (loop :with last-good := first ; Keep the last good key :for the-cons :on (cddr first) :by #'cddr :do (progn (assert-proper-plist (cdr the-cons)) (if (bad-key-p (car the-cons)) (setf (cddr last-good) (cddr the-cons)) (setf last-good the-cons))) :finally (return first)))))
Using two loops seems awkward to me. How about one?
(defun delete-from-plist (plist &rest keys) (loop with head = plist with tail = nil for (key . rest) on plist by #'cddr do (assert rest () "Expected a proper plist, got ~S" plist) (if (member key keys :test #'eq) (let ((next (cdr rest))) (if tail (setf (cdr tail) next) (setf head next))) (setf tail rest)) finally (return head)))
"James M. Lawrence" llmjjmll@gmail.com writes:
Using two loops seems awkward to me. How about one?
(defun delete-from-plist (plist &rest keys) (loop with head = plist with tail = nil for (key . rest) on plist by #'cddr do (assert rest () "Expected a proper plist, got ~S" plist) (if (member key keys :test #'eq) (let ((next (cdr rest))) (if tail (setf (cdr tail) next) (setf head next))) (setf tail rest)) finally (return head)))
I mention this for completeness and novelty, not for suitability:
(defun sans (plist &rest keys) (let ((sans ())) (loop (let ((tail (nth-value 2 (get-properties plist keys)))) ;; this is how it ends (unless tail (return (nreconc sans plist))) ;; copy all the unmatched keys (loop until (eq plist tail) do (push (pop plist) sans) (push (pop plist) sans)) ;; skip the matched key (setq plist (cddr plist))))))
I don't think I've seen GET-PROPERTIES and NRECONC outside this function.
I got it from here: http://xach.com/naggum/articles/3247672165664225%40naggum.no.html
Zach
Zach Beane xach@xach.com writes:
"James M. Lawrence" llmjjmll@gmail.com writes:
Using two loops seems awkward to me. How about one?
(defun delete-from-plist (plist &rest keys) (loop with head = plist with tail = nil for (key . rest) on plist by #'cddr do (assert rest () "Expected a proper plist, got ~S" plist) (if (member key keys :test #'eq) (let ((next (cdr rest))) (if tail (setf (cdr tail) next) (setf head next))) (setf tail rest)) finally (return head)))
I mention this for completeness and novelty, not for suitability:
(defun sans (plist &rest keys) (let ((sans ())) (loop (let ((tail (nth-value 2 (get-properties plist keys)))) ;; this is how it ends (unless tail (return (nreconc sans plist))) ;; copy all the unmatched keys (loop until (eq plist tail) do (push (pop plist) sans) (push (pop plist) sans)) ;; skip the matched key (setq plist (cddr plist))))))
I don't think I've seen GET-PROPERTIES and NRECONC outside this function.
I got it from here: http://xach.com/naggum/articles/3247672165664225%40naggum.no.html
If striving for shortness:
(defun delete-from-plist (plist &rest keys) (dolist (key keys plist) (remf plist key)))
Looks good! Much cleaner/better.
-Robert
On Sun, Feb 24, 2013 at 6:30 AM, James M. Lawrence llmjjmll@gmail.com wrote:
On Sat, Feb 23, 2013 at 4:09 AM, Robert Smith quad@symbo1ics.com wrote:
(defun delete-from-plist (plist &rest keys) "Delete all keys and pairs indicated by KEYS from the plist PLIST." (labels ((assert-proper-plist (x) (assert x () "Expected a proper plist, got ~S" plist)) (bad-key-p (key) (member key keys :test #'eq)) (find-first () "Find the first cons in PLIST to keep." (loop :for the-cons :on plist :by #'cddr :unless (prog1 (bad-key-p (car the-cons)) (assert-proper-plist (cdr the-cons))) :do (return the-cons) :finally (return nil)))) (declare (inline assert-proper-plist bad-key-p find-first)) ;; Find the first good key and delete any bad key-value pairs ;; between it and the start. (let ((first (find-first))) (unless (eq first plist) (setf (cddr plist) first))
;; At this point, we know FIRST points to the first key ;; which exists, or NIL. (loop :with last-good := first ; Keep the last good key :for the-cons :on (cddr first) :by #'cddr :do (progn (assert-proper-plist (cdr the-cons)) (if (bad-key-p (car the-cons)) (setf (cddr last-good) (cddr the-cons)) (setf last-good the-cons))) :finally (return first)))))Using two loops seems awkward to me. How about one?
(defun delete-from-plist (plist &rest keys) (loop with head = plist with tail = nil for (key . rest) on plist by #'cddr do (assert rest () "Expected a proper plist, got ~S" plist) (if (member key keys :test #'eq) (let ((next (cdr rest))) (if tail (setf (cdr tail) next) (setf head next))) (setf tail rest)) finally (return head)))
alexandria-devel mailing list alexandria-devel@common-lisp.net http://lists.common-lisp.net/cgi-bin/mailman/listinfo/alexandria-devel
pushed, thanks!
and sorry for the "delay"!
On Tue, Mar 4, 2014 at 2:43 AM, Attila Lendvai attila.lendvai@gmail.com wrote:
pushed, thanks!
and sorry for the "delay"!
I just noticed the function I wrote was replaced with
(dolist (key keys plist) (remf plist key))
but that does not match the behavior of remove-from-plist,
CL-USER> (alexandria:remove-from-plist (list :a 1 :b 2 :a 3) :a) (:B 2) CL-USER> (alexandria:delete-from-plist (list :a 1 :b 2 :a 3) :a) (:B 2 :A 3)
It also don't check for malformed plists. The commit message says the remf version is faster, but I found it to be around four times slower for some data and insignificantly different otherwise.
I just noticed the function I wrote was replaced with
(dolist (key keys plist) (remf plist key))
but that does not match the behavior of remove-from-plist,
CL-USER> (alexandria:remove-from-plist (list :a 1 :b 2 :a 3) :a) (:B 2) CL-USER> (alexandria:delete-from-plist (list :a 1 :b 2 :a 3) :a) (:B 2 :A 3)
It also don't check for malformed plists. The commit message says the remf version is faster, but I found it to be around four times slower for some data and insignificantly different otherwise.
pushed (once again blindly beliving the patch author).
but this time i also pushed a test at lest that tests for the removal of duplicate keys.
sorry for blindly pushing the previous change.
alexandria-devel@common-lisp.net
-
Attila Lendvai -
James M. Lawrence -
Robert Smith -
Stas Boukarev -
Zach Beane