Clojure

Add queue and queue? to clojure.core

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: Release 1.5
  • Fix Version/s: None
  • Component/s: None
  • Patch:
    Code and Test
  • Approval:
    Triaged

Description

Add queue function to create queues from collections and queue? predicate to check queueness.

Patch: clj-1048-add-queue-functions.diff

  1. clj-1048-add-queue-functions.diff
    05/Feb/14 6:53 PM
    5 kB
    John Jacobsen
  2. queue.patch
    26/Oct/12 9:16 PM
    6 kB
    Timothy Baldridge

Activity

Hide
Andy Fingerhut added a comment -

Timothy, I tried applying both of these Sep 26, 2012 patches to latest Clojure master as of that date. I had to apply 0001-make-PersistentQueue-ctor-public.patch by hand since it failed to apply using git or patch. It built fine, but failed to pass several of the Clojure tests. Have you looked into those test failures to see if you can find the cause and fix them? I tested on Ubuntu 11.10 with Oracle JDK 1.6 and 1.7, and saw similar failures with both.

Show
Andy Fingerhut added a comment - Timothy, I tried applying both of these Sep 26, 2012 patches to latest Clojure master as of that date. I had to apply 0001-make-PersistentQueue-ctor-public.patch by hand since it failed to apply using git or patch. It built fine, but failed to pass several of the Clojure tests. Have you looked into those test failures to see if you can find the cause and fix them? I tested on Ubuntu 11.10 with Oracle JDK 1.6 and 1.7, and saw similar failures with both.
Hide
Timothy Baldridge added a comment -

Fixed the patch. Tests pass, created the patch, applied it to a different copy of the source and the tests still pass. So this new patch should be good to go.

Show
Timothy Baldridge added a comment - Fixed the patch. Tests pass, created the patch, applied it to a different copy of the source and the tests still pass. So this new patch should be good to go.
Timothy Baldridge made changes -
Field Original Value New Value
Attachment queues.patch [ 11621 ]
Hide
Andy Fingerhut added a comment -

Timothy, I'm not sure how you are getting successful results when applying this patch. Can you try the steps below and see what happens for you? I get errors trying to apply the patch with latest Clojure master as of Oct 26, 2012. Also please use the steps on the JIRA workflow page to create a git format patch (http://dev.clojure.org/display/design/JIRA+workflow under "Development" heading).

% git clone git://github.com/clojure/clojure.git
% cd clojure
% patch -p1 < queues.patch
patching file src/clj/clojure/core.clj
patching file src/jvm/clojure/lang/PersistentQueue.java
Hunk #1 FAILED at 32.
1 out of 1 hunk FAILED – saving rejects to file src/jvm/clojure/lang/PersistentQueue.java.rej
patching file test/clojure/test_clojure/data_structures.clj
Hunk #1 succeeded at 123 with fuzz 2.
Hunk #2 succeeded at 861 with fuzz 2.
Hunk #3 FAILED at 872.
1 out of 3 hunks FAILED – saving rejects to file test/clojure/test_clojure/data_structures.clj.rej
patching file test/clojure/test_clojure/java_interop.clj

Show
Andy Fingerhut added a comment - Timothy, I'm not sure how you are getting successful results when applying this patch. Can you try the steps below and see what happens for you? I get errors trying to apply the patch with latest Clojure master as of Oct 26, 2012. Also please use the steps on the JIRA workflow page to create a git format patch (http://dev.clojure.org/display/design/JIRA+workflow under "Development" heading). % git clone git://github.com/clojure/clojure.git % cd clojure % patch -p1 < queues.patch patching file src/clj/clojure/core.clj patching file src/jvm/clojure/lang/PersistentQueue.java Hunk #1 FAILED at 32. 1 out of 1 hunk FAILED – saving rejects to file src/jvm/clojure/lang/PersistentQueue.java.rej patching file test/clojure/test_clojure/data_structures.clj Hunk #1 succeeded at 123 with fuzz 2. Hunk #2 succeeded at 861 with fuzz 2. Hunk #3 FAILED at 872. 1 out of 3 hunks FAILED – saving rejects to file test/clojure/test_clojure/data_structures.clj.rej patching file test/clojure/test_clojure/java_interop.clj
Hide
Timothy Baldridge added a comment -

I was using git apply. I tried the method you show above, and now I'm seeing the same issues you show above.

Show
Timothy Baldridge added a comment - I was using git apply. I tried the method you show above, and now I'm seeing the same issues you show above.
Hide
Andy Fingerhut added a comment -

Just so you know, the preferred way to create and apply patches are the "git format-patch master --stdout > patch.txt" to create a patch (after doing the branching commands described on the JIRA workflow page to create a branch for your changes), and the "git am --keep-cr -s < patch.txt" to apply a patch. If a patch was created that way and applies cleanly with that command, then you are definitely good to go.

The "patch -p1 < patch.txt" command is just a secondary method sometimes used to try to apply patches that aren't in the format produced above, or have errors when applying using that method.

Show
Andy Fingerhut added a comment - Just so you know, the preferred way to create and apply patches are the "git format-patch master --stdout > patch.txt" to create a patch (after doing the branching commands described on the JIRA workflow page to create a branch for your changes), and the "git am --keep-cr -s < patch.txt" to apply a patch. If a patch was created that way and applies cleanly with that command, then you are definitely good to go. The "patch -p1 < patch.txt" command is just a secondary method sometimes used to try to apply patches that aren't in the format produced above, or have errors when applying using that method.
Hide
Timothy Baldridge added a comment -

Just so you know, the preferred way to create and apply patches are the "git format-patch master --stdout > patch.txt" to create a patch (after doing the branching commands described on the JIRA workflow page to create a branch for your changes), and the "git am --keep-cr -s < patch.txt" to apply a patch. If a patch was created that way and applies cleanly with that command, then you are definitely good to go.

The "patch -p1 < patch.txt" command is just a secondary method sometimes used to try to apply patches that aren't in the format produced above, or have errors when applying using that method.

Show
Timothy Baldridge added a comment - Just so you know, the preferred way to create and apply patches are the "git format-patch master --stdout > patch.txt" to create a patch (after doing the branching commands described on the JIRA workflow page to create a branch for your changes), and the "git am --keep-cr -s < patch.txt" to apply a patch. If a patch was created that way and applies cleanly with that command, then you are definitely good to go. The "patch -p1 < patch.txt" command is just a secondary method sometimes used to try to apply patches that aren't in the format produced above, or have errors when applying using that method.
Hide
Timothy Baldridge added a comment - - edited

added patch

Show
Timothy Baldridge added a comment - - edited added patch
Timothy Baldridge made changes -
Attachment queue.patch [ 11622 ]
Timothy Baldridge made changes -
Attachment 0001-make-PersistentQueue-ctor-public.patch [ 11527 ]
Timothy Baldridge made changes -
Attachment queues.patch [ 11621 ]
Timothy Baldridge made changes -
Attachment 0001-added-queue-queue-and-queue.patch [ 11526 ]
Hide
Andy Fingerhut added a comment -

That one applies cleanly and passes all tests. It should show up on the next list of prescreened patches. Thanks.

Show
Andy Fingerhut added a comment - That one applies cleanly and passes all tests. It should show up on the next list of prescreened patches. Thanks.
Hide
Rich Hickey added a comment -

we don't use the queue* convention elsewhere, e.g. vec and vector. I think queue should take a collection like vec and set. (queue [1 2 3]) could be made to 'adopt' the collection as front.

Show
Rich Hickey added a comment - we don't use the queue* convention elsewhere, e.g. vec and vector. I think queue should take a collection like vec and set. (queue [1 2 3]) could be made to 'adopt' the collection as front.
Hide
Andy Fingerhut added a comment -

Patch queue.patch dated Oct 26 2012 no longer applies cleanly after recent CLJ-1000 commits, but only because of one line of changed patch context. It still applies cleanly with "patch -p1 < queue.patch". Not bothering to update the stale patch given Rich's comments suggesting more substantive changes.

Show
Andy Fingerhut added a comment - Patch queue.patch dated Oct 26 2012 no longer applies cleanly after recent CLJ-1000 commits, but only because of one line of changed patch context. It still applies cleanly with "patch -p1 < queue.patch". Not bothering to update the stale patch given Rich's comments suggesting more substantive changes.
Hide
Steve Miner added a comment -

See also CLJ-976 (tagged literal support for PersistentQueue)

Show
Steve Miner added a comment - See also CLJ-976 (tagged literal support for PersistentQueue)
Hide
John Jacobsen added a comment -

Don't want to step on Timothy B's toes here, but it looks straightforward to adopt his patch to implement Rich's suggestion. I'd offer to give it a whack if nobody else wants the ticket now.

Show
John Jacobsen added a comment - Don't want to step on Timothy B's toes here, but it looks straightforward to adopt his patch to implement Rich's suggestion. I'd offer to give it a whack if nobody else wants the ticket now.
Hide
John Jacobsen added a comment -
Show
John Jacobsen added a comment - Discussion initiated on clojure-dev: https://groups.google.com/forum/?fromgroups#!topic/clojure-dev/2BOqHm24Vc4
Hide
John Jacobsen added a comment -

This patch (if accepted) supersedes Timothy Baldridge's patch; it implements "queue" and "queue?" (but not "queue*"); "queue" accepts a collection rather than being a variadic function, as per Rich's suggestion.

Show
John Jacobsen added a comment - This patch (if accepted) supersedes Timothy Baldridge's patch; it implements "queue" and "queue?" (but not "queue*"); "queue" accepts a collection rather than being a variadic function, as per Rich's suggestion.
John Jacobsen made changes -
Attachment clj-1048-queue-takes-collections.diff [ 12018 ]
John Jacobsen made changes -
Description This patch adds functions for PersistentQueue. queue, queue? and queue* match the list functions of the same naming conventions. Patches include updates to tests. Original patch added functions for PersistentQueue. queue, queue? and queue* match the list functions of the same naming conventions. Patches include updates to tests.

In the updated patch, "queue" accepts a single collection argument as per Rich's suggestion, and does not provide a "queue*".
Hide
Andy Fingerhut added a comment -

The patch clj-1048-queue-takes-collections.diff applied cleanly to latest Clojure master as of Jan 23 2014, but not on Jan 30 2014. There were several commits made to Clojure during that week involving updating the hash functions that conflict in some way with this patch. I have not checked to see how easy or difficult it might be to update the patch.

Show
Andy Fingerhut added a comment - The patch clj-1048-queue-takes-collections.diff applied cleanly to latest Clojure master as of Jan 23 2014, but not on Jan 30 2014. There were several commits made to Clojure during that week involving updating the hash functions that conflict in some way with this patch. I have not checked to see how easy or difficult it might be to update the patch.
John Jacobsen made changes -
Attachment clj-1048-queue-takes-collections-1-6.diff [ 12761 ]
John Jacobsen made changes -
Attachment clj-1048-queue-takes-collections.diff [ 12018 ]
Hide
John Jacobsen added a comment -

Hi Andy, I updated the patch and removed my previous version. The new one should apply cleanly and pass all tests.

Show
John Jacobsen added a comment - Hi Andy, I updated the patch and removed my previous version. The new one should apply cleanly and pass all tests.
John Jacobsen made changes -
Comment [ This version applies cleanly to the current repo and all tests pass. ]
Hide
John Jacobsen added a comment -

Updated ticket title.

Show
John Jacobsen added a comment - Updated ticket title.
John Jacobsen made changes -
Summary Added queue, queue* and queue? to clojure.core Added queue and queue? to clojure.core
John Jacobsen made changes -
Summary Added queue and queue? to clojure.core Add queue and queue? to clojure.core
Alex Miller made changes -
Description Original patch added functions for PersistentQueue. queue, queue? and queue* match the list functions of the same naming conventions. Patches include updates to tests.

In the updated patch, "queue" accepts a single collection argument as per Rich's suggestion, and does not provide a "queue*".
Add {{queue}} function to create queues from collections and {{queue?}} predicate to check queueness.

*Patch:* clj-1048-queue-takes-collections-1-6.diff
Alex Miller made changes -
Approval Triaged [ 10120 ]
Priority Trivial [ 5 ] Minor [ 4 ]
Hide
Alex Miller added a comment -

Hi John... Can you condense these changes into a single commit? Please also remove the comments above queue* in java_interop.clj. Thanks...

Show
Alex Miller added a comment - Hi John... Can you condense these changes into a single commit? Please also remove the comments above queue* in java_interop.clj. Thanks...
John Jacobsen made changes -
Attachment clj-1048-add-queue-functions.diff [ 12764 ]
John Jacobsen made changes -
Attachment clj-1048-queue-takes-collections-1-6.diff [ 12761 ]
Hide
John Jacobsen added a comment -

Hi Alex, the updated patch removes that comment and rebases all three commits into c9f77dd. Let me know if you need anything else. Thanks!

Show
John Jacobsen added a comment - Hi Alex, the updated patch removes that comment and rebases all three commits into c9f77dd. Let me know if you need anything else. Thanks!
Alex Miller made changes -
Description Add {{queue}} function to create queues from collections and {{queue?}} predicate to check queueness.

*Patch:* clj-1048-queue-takes-collections-1-6.diff
Add {{queue}} function to create queues from collections and {{queue?}} predicate to check queueness.

*Patch:* clj-1048-add-queue-functions.diff
Hide
Bozhidar Batsov added a comment -

A tiny remark - I think the docstrings should end with a dot.

Show
Bozhidar Batsov added a comment - A tiny remark - I think the docstrings should end with a dot.

People

Vote (9)
Watch (4)

Dates

  • Created:
    Updated: