Clojure

Allow ISeq args to map conj

Details

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

Description

It's confusing to users why vectors are allowed while sequences (which compare `=` to a vector) aren't.

Currently conj on maps can take only maps or MapEntries, this patch allows it to take any ISeq:

user=> (conj {} '(1 2))
{1 2}

Activity

Hide
Alex Miller added a comment -

Why? Please start the ticket with the problem you are trying out to solve.

Show
Alex Miller added a comment - Why? Please start the ticket with the problem you are trying out to solve.
Hide
Nicola Mometto added a comment -

It's confusing to users why vectors are allowed while sequences (which compare `=` to a vector) aren't.

Show
Nicola Mometto added a comment - It's confusing to users why vectors are allowed while sequences (which compare `=` to a vector) aren't.
Hide
Nicolas HA added a comment -

Not sure if that was the original intent, but I came across this because a friend asking me why this doesn't work as expected, and I could not tell :

> (conj {:a 1} [:b 2] [:c 3])
{:c 3, :b 2, :a 1}
> (conj {:a 1} '(:b 2) [:c 3])
java.lang.ClassCastException: clojure.lang.Keyword cannot be cast to java.util.Map$Entry

Show
Nicolas HA added a comment - Not sure if that was the original intent, but I came across this because a friend asking me why this doesn't work as expected, and I could not tell :
> (conj {:a 1} [:b 2] [:c 3]) {:c 3, :b 2, :a 1} > (conj {:a 1} '(:b 2) [:c 3]) java.lang.ClassCastException: clojure.lang.Keyword cannot be cast to java.util.Map$Entry
Hide
Alex Miller added a comment -

Map entries require fast lookup for key and value. MapEntry supports this via fields. Vectors support this via indexed lookup. Seqs and lists can't support this.

Show
Alex Miller added a comment - Map entries require fast lookup for key and value. MapEntry supports this via fields. Vectors support this via indexed lookup. Seqs and lists can't support this.
Hide
Nicola Mometto added a comment -

conjoining a seq to a map would still be faster than conjoining a map to a map, which is a supported operation so I don't see how the perf concern matters

(conj {} '(1 2))
Evaluation count : 574987740 in 60 samples of 9583129 calls.
             Execution time mean : 106.172816 ns
    Execution time std-deviation : 3.859530 ns
   Execution time lower quantile : 98.791209 ns ( 2.5%)
   Execution time upper quantile : 113.382563 ns (97.5%)
                   Overhead used : 1.557920 ns

Found 1 outliers in 60 samples (1.6667 %)
	low-severe	 1 (1.6667 %)
 Variance from outliers : 22.2604 % Variance is moderately inflated by outliers
(conj {} [1 2])
Evaluation count : 1387428240 in 60 samples of 23123804 calls.
             Execution time mean : 38.640388 ns
    Execution time std-deviation : 4.792049 ns
   Execution time lower quantile : 31.823105 ns ( 2.5%)
   Execution time upper quantile : 46.920771 ns (97.5%)
                   Overhead used : 1.557920 ns
(conj {} {1 2})
Evaluation count : 359285400 in 60 samples of 5988090 calls.
             Execution time mean : 156.084015 ns
    Execution time std-deviation : 8.769030 ns
   Execution time lower quantile : 144.896787 ns ( 2.5%)
   Execution time upper quantile : 170.623058 ns (97.5%)
                   Overhead used : 1.557920 ns
Show
Nicola Mometto added a comment - conjoining a seq to a map would still be faster than conjoining a map to a map, which is a supported operation so I don't see how the perf concern matters
(conj {} '(1 2))
Evaluation count : 574987740 in 60 samples of 9583129 calls.
             Execution time mean : 106.172816 ns
    Execution time std-deviation : 3.859530 ns
   Execution time lower quantile : 98.791209 ns ( 2.5%)
   Execution time upper quantile : 113.382563 ns (97.5%)
                   Overhead used : 1.557920 ns

Found 1 outliers in 60 samples (1.6667 %)
	low-severe	 1 (1.6667 %)
 Variance from outliers : 22.2604 % Variance is moderately inflated by outliers
(conj {} [1 2])
Evaluation count : 1387428240 in 60 samples of 23123804 calls.
             Execution time mean : 38.640388 ns
    Execution time std-deviation : 4.792049 ns
   Execution time lower quantile : 31.823105 ns ( 2.5%)
   Execution time upper quantile : 46.920771 ns (97.5%)
                   Overhead used : 1.557920 ns
(conj {} {1 2})
Evaluation count : 359285400 in 60 samples of 5988090 calls.
             Execution time mean : 156.084015 ns
    Execution time std-deviation : 8.769030 ns
   Execution time lower quantile : 144.896787 ns ( 2.5%)
   Execution time upper quantile : 170.623058 ns (97.5%)
                   Overhead used : 1.557920 ns
Hide
Jozef Wagner added a comment -

I got bitten by this when processing data from fressian, that returns j.u.List instead of c.l.IPersistentVector. Had to use a custom version of into for that.

(defn into-map
  "Like (into {} coll) but always assumes coll items
  are [key value] pairs."
  [coll]
  (let [rf (fn [m [k v]] (assoc! m k v))]
    (persistent! (reduce rf (transient {}) coll))))

With above patch, I could just use

(into {} (map seq coll))
Show
Jozef Wagner added a comment - I got bitten by this when processing data from fressian, that returns j.u.List instead of c.l.IPersistentVector. Had to use a custom version of into for that.
(defn into-map
  "Like (into {} coll) but always assumes coll items
  are [key value] pairs."
  [coll]
  (let [rf (fn [m [k v]] (assoc! m k v))]
    (persistent! (reduce rf (transient {}) coll))))
With above patch, I could just use
(into {} (map seq coll))

People

Vote (1)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: