Clojure

subseq, rsubseq enhancements to support priority maps?

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: Backlog
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test

Description

See dev thread at http://groups.google.com/group/clojure-dev/browse_thread/thread/fdb000cae4f66a95.

Note: subseq currently returns () instead of nil in some situations. If the rest of this idea dies we might still want to fix that.

Activity

Hide
Assembla Importer added a comment -
Show
Assembla Importer added a comment - Converted from http://www.assembla.com/spaces/clojure/tickets/428
Aaron Bedra made changes -
Field Original Value New Value
Fix Version/s Approved Backlog [ 10034 ]
Fix Version/s Release.Next [ 10038 ]
Reporter Assembla Importer [ importer ]
Affects Version/s Approved Backlog [ 10034 ]
Priority Blocker [ 1 ]
Aaron Bedra made changes -
Priority Blocker [ 1 ] Minor [ 4 ]
Hide
Andy Fingerhut added a comment -

Patch clj-428-change-Sorted-seqFrom-to-take-inclusive-patch-v1.txt dated Apr 28 2013 was written by Mark Engelberg in July 2010, and was attached to a message he sent to the dev thread linked in the description. The approach he takes is described by him in that thread, copied here:

----------------------------------------
Meanwhile, to initiate discussion on how to modify subseq, I've attached a proposed patch. This patch works by modifying the seqFrom method of the Sorted interface to take an additional "inclusive" parameter (i.e., <= and >= are inclusive, < and > are not).

In this patch, I do not address one issue I raised before, which is whether subseq implies by its name that it should return a seq rather than a sequence (in other words nil rather than ()). If seq behavior is desired, it would be necessary to wrap a call to seq around the calls to take-while. But for now, I'm just making the behavior match the current behavior.

Although I think this is the cleanest way to address the extensibility issue with subseq, the change to seqFrom will break anyone who currently is overriding that method. I didn't see any such classes in clojure.contrib, so I don't think it's an issue, but if this is a concern, my other idea is to fix the problem entirely within subseq by changing the calls to next into calls to drop-while. I have coded this to confirm that it works, and can provide that alternative patch if desired.
----------------------------------------

I can also supply a patch that uses drop-while in clojure.core/subseq and rsubseq if such an approach is preferred to the one in this patch.

Show
Andy Fingerhut added a comment - Patch clj-428-change-Sorted-seqFrom-to-take-inclusive-patch-v1.txt dated Apr 28 2013 was written by Mark Engelberg in July 2010, and was attached to a message he sent to the dev thread linked in the description. The approach he takes is described by him in that thread, copied here: ---------------------------------------- Meanwhile, to initiate discussion on how to modify subseq, I've attached a proposed patch. This patch works by modifying the seqFrom method of the Sorted interface to take an additional "inclusive" parameter (i.e., <= and >= are inclusive, < and > are not). In this patch, I do not address one issue I raised before, which is whether subseq implies by its name that it should return a seq rather than a sequence (in other words nil rather than ()). If seq behavior is desired, it would be necessary to wrap a call to seq around the calls to take-while. But for now, I'm just making the behavior match the current behavior. Although I think this is the cleanest way to address the extensibility issue with subseq, the change to seqFrom will break anyone who currently is overriding that method. I didn't see any such classes in clojure.contrib, so I don't think it's an issue, but if this is a concern, my other idea is to fix the problem entirely within subseq by changing the calls to next into calls to drop-while. I have coded this to confirm that it works, and can provide that alternative patch if desired. ---------------------------------------- I can also supply a patch that uses drop-while in clojure.core/subseq and rsubseq if such an approach is preferred to the one in this patch.
Andy Fingerhut made changes -
Attachment clj-428-change-Sorted-seqFrom-to-take-inclusive-patch-v1.txt [ 11970 ]
Andy Fingerhut made changes -
Patch Code [ 10001 ]
Hide
Andy Fingerhut added a comment -

clj-428-change-Sorted-seqFrom-to-take-inclusive-patch-v2.txt dated Apr 28 2013 is same as clj-428-change-Sorted-seqFrom-to-take-inclusive-patch-v1.txt (soon to be deleted), except it adds tests for subseq and rsubseq, and corrects a bug in that patch. The approach is the same as described above for that patch.

Show
Andy Fingerhut added a comment - clj-428-change-Sorted-seqFrom-to-take-inclusive-patch-v2.txt dated Apr 28 2013 is same as clj-428-change-Sorted-seqFrom-to-take-inclusive-patch-v1.txt (soon to be deleted), except it adds tests for subseq and rsubseq, and corrects a bug in that patch. The approach is the same as described above for that patch.
Andy Fingerhut made changes -
Attachment clj-428-change-Sorted-seqFrom-to-take-inclusive-patch-v2.txt [ 11973 ]
Andy Fingerhut made changes -
Attachment clj-428-change-Sorted-seqFrom-to-take-inclusive-patch-v1.txt [ 11970 ]
Hide
Andy Fingerhut added a comment -

Patch clj-428-change-Sorted-seqFrom-to-take-inclusive-patch-v3.txt dated May 1 2013 is the same as clj-428-change-Sorted-seqFrom-to-take-inclusive-patch-v1.txt, still with the bug fix mentioned for -v2, but with some unnecessary changes removed from the patch. The comments for -v1.txt on the approach still apply.

Show
Andy Fingerhut added a comment - Patch clj-428-change-Sorted-seqFrom-to-take-inclusive-patch-v3.txt dated May 1 2013 is the same as clj-428-change-Sorted-seqFrom-to-take-inclusive-patch-v1.txt, still with the bug fix mentioned for -v2, but with some unnecessary changes removed from the patch. The comments for -v1.txt on the approach still apply.
Andy Fingerhut made changes -
Attachment clj-428-change-Sorted-seqFrom-to-take-inclusive-patch-v3.txt [ 11977 ]
Andy Fingerhut made changes -
Attachment clj-428-change-Sorted-seqFrom-to-take-inclusive-patch-v2.txt [ 11973 ]
Alex Miller made changes -
Fix Version/s Approved Backlog [ 10034 ]
Fix Version/s Backlog [ 10035 ]
Alex Miller made changes -
Affects Version/s Approved Backlog [ 10034 ]
Affects Version/s Backlog [ 10035 ]
Alex Miller made changes -
Fix Version/s Backlog [ 10035 ]
Hide
Andy Fingerhut added a comment -

Patch clj-428-v4.diff is identical to clj-428-change-Sorted-seqFrom-to-take-inclusive-patch-v3.txt described in an earlier comment, except it updates some diff context lines so that it applies cleanly to the latest Clojure master as of today.

Show
Andy Fingerhut added a comment - Patch clj-428-v4.diff is identical to clj-428-change-Sorted-seqFrom-to-take-inclusive-patch-v3.txt described in an earlier comment, except it updates some diff context lines so that it applies cleanly to the latest Clojure master as of today.
Andy Fingerhut made changes -
Attachment clj-428-v4.diff [ 12793 ]
Andy Fingerhut made changes -
Attachment clj-428-change-Sorted-seqFrom-to-take-inclusive-patch-v3.txt [ 11977 ]
Andy Fingerhut made changes -
Patch Code [ 10001 ] Code and Test [ 10002 ]

People

Vote (2)
Watch (3)

Dates

  • Created:
    Updated: