Clojure

Stop ISeq from inheriting Sequential

Details

  • Type: Task Task
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: Release 1.3
  • Component/s: None
  • Labels:
    None
  • Approval:
    Ok

Description

Currently, ISeq extends Sequential, which is what collections like vector use to determine equality partition membership. This prevents anything that implements ISeq from being exclusively in the map or set equality partition.

"Care will be required in making sure all appropriate concrete derivees remain Sequential. It's a breaking change in that some derivees not in Clojure may be relying on the derivation from ISeq"

http://groups.google.com/group/clojure-dev/browse_thread/thread/c58ec42d78209ee0

Activity

Hide
Chouser added a comment -

Here are the classes I found that implement ISeq and thus will need to be changed to implement Sequential:

  • LazySeq
  • ASeq
    • Range
    • EnumerationSeq
    • ArraySeq (including primitives)
    • IteratorSeq
    • Cons
    • StringSeq
    • ChunkedCons
    • PersistentList
    • PersistentVector Seq, RSeq and ChunkedSeq
    • PersistentMap KeySeq and ValSeq
    • PersistentHashMap Seq and NodeSeq
    • PersistentQueue Seq
    • PersistentTreeMap Seq
    • PersistentArrayMap Seq
    • PersistentStructMap Seq
  • IndexedSeq
    • ArraySeq (including primitives)
    • StringSeq
    • PersistentVector Seq and RSeq
  • IChunkedSeq
    • ChunkedCons
    • PersistentVector ChunkedSeq
    • VecSeq (from gvec)

Should ASeq, IndexedSeq, or IChunkedSeq implement Sequential in order to impart that to all their derived classes? My initial thought is that ASeq should reflect only ISeq and so shouldn't implement Sequential. But I find it hard to imagine indexed or chunked seqs that aren't sequential. Any thoughts?

Also, print-method currently prints all ISeqs with round parens – it seems to me this should be changed to test for Sequential instead. Does anyone disagree?

Show
Chouser added a comment - Here are the classes I found that implement ISeq and thus will need to be changed to implement Sequential:
  • LazySeq
  • ASeq
    • Range
    • EnumerationSeq
    • ArraySeq (including primitives)
    • IteratorSeq
    • Cons
    • StringSeq
    • ChunkedCons
    • PersistentList
    • PersistentVector Seq, RSeq and ChunkedSeq
    • PersistentMap KeySeq and ValSeq
    • PersistentHashMap Seq and NodeSeq
    • PersistentQueue Seq
    • PersistentTreeMap Seq
    • PersistentArrayMap Seq
    • PersistentStructMap Seq
  • IndexedSeq
    • ArraySeq (including primitives)
    • StringSeq
    • PersistentVector Seq and RSeq
  • IChunkedSeq
    • ChunkedCons
    • PersistentVector ChunkedSeq
    • VecSeq (from gvec)
Should ASeq, IndexedSeq, or IChunkedSeq implement Sequential in order to impart that to all their derived classes? My initial thought is that ASeq should reflect only ISeq and so shouldn't implement Sequential. But I find it hard to imagine indexed or chunked seqs that aren't sequential. Any thoughts? Also, print-method currently prints all ISeqs with round parens – it seems to me this should be changed to test for Sequential instead. Does anyone disagree?
Hide
Rich Hickey added a comment -

> Should ASeq, IndexedSeq, or IChunkedSeq implement Sequential in order to impart that to all their derived classes?

Yes, all.

> Also, print-method currently prints all ISeqs with round parens – it seems to me this should be changed to test for Sequential instead. Does anyone disagree?

Sounds ok.

Show
Rich Hickey added a comment - > Should ASeq, IndexedSeq, or IChunkedSeq implement Sequential in order to impart that to all their derived classes? Yes, all. > Also, print-method currently prints all ISeqs with round parens – it seems to me this should be changed to test for Sequential instead. Does anyone disagree? Sounds ok.
Hide
Chouser added a comment -

This patch does not change the print-method – it turns out that would have been a mistake.

Show
Chouser added a comment - This patch does not change the print-method – it turns out that would have been a mistake.
Hide
Stuart Halloway added a comment -

My patch is same as Chouser's except tweaked to make the git gods happy.

Show
Stuart Halloway added a comment - My patch is same as Chouser's except tweaked to make the git gods happy.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: