Support more data types in keys and vals (#1017)

Fixes #1018 
Fixes #1020

Author: chrisrink10

CHANGELOG.md

@@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
  * Fix the behaviour of `nil` with several collection functions (#1011)
+ * Fix a bug where `keys` and `vals` did not yield keys and values from sequences of map entries (#1018)
+ * Fix a bug where `set` and `vec` do not produce collections of map entries when called on map arguments (#1020)
 
 ## [v0.2.0]
 ### Added

src/basilisp/core.lpy

@@ -246,7 +246,7 @@
   set
   (fn set
     [coll]
-    (^:allow-builtins basilisp.lang.set/set coll)))
+    (^:allow-builtins basilisp.lang.runtime/to-set coll)))
 
 (def
   ^{:doc      "Create a vector from the input arguments."
@@ -261,7 +261,7 @@
   vec
   (fn vec
     [coll]
-    (basilisp.lang.vector/vector coll)))
+    (basilisp.lang.runtime/vector coll)))
 
 ;;;;;;;;;;;; full support for syntax quote begins here ;;;;;;;;;;;;
 
@@ -332,8 +332,8 @@
 (.alter-meta #'vector? assoc :inline (fn [o] `(instance? basilisp.lang.interfaces/IPersistentVector ~o)))
 (.alter-meta #'seq? assoc :inline (fn [o] `(instance? basilisp.lang.interfaces/ISeq ~o)))
 (.alter-meta #'seq assoc :inline (fn [o] `(basilisp.lang.runtime/to-seq ~o)))
-(.alter-meta #'set assoc :inline (fn [coll] `(basilisp.lang.set/set ~coll)))
-(.alter-meta #'vec assoc :inline (fn [coll] `(basilisp.lang.vector/vector ~coll)))
+(.alter-meta #'set assoc :inline (fn [coll] `(basilisp.lang.runtime/to-set ~coll)))
+(.alter-meta #'vec assoc :inline (fn [coll] `(basilisp.lang.runtime/vector ~coll)))
 
 (def
   ^{:macro    true
@@ -2322,14 +2322,32 @@
   (.-value entry))
 
 (defn ^:inline keys
-  "Return a seq of the keys from a map."
+  "Return a seq of the keys from a map-like object.
+
+  If ``m`` is ``nil``, return ``nil``.
+
+  If ``m`` is castable to a seq, cast it to a seq and call ``keys`` on that.
+
+  If ``m`` is a seq, yield the keys from successive entries (which must be map
+  entries).
+
+  If ``m`` is any type of mapping, return a seq of that mapping's keys."
   [m]
-  (when m (seq (.keys m))))
+  (basilisp.lang.runtime/keys m))
 
 (defn ^:inline vals
-  "Return a seq of the values from a map."
+  "Return a seq of the values from a map-like object
+
+  If ``m`` is ``nil``, return ``nil``.
+
+  If ``m`` is castable to a seq, cast it to a seq and call ``vals`` on that.
+
+  If ``m`` is a seq, yield the valuess from successive entries (which must be map
+  entries).
+
+  If ``m`` is any type of mapping, return a seq of that mapping's values."
   [m]
-  (when m (seq (.values m))))
+  (basilisp.lang.runtime/vals m))
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;; Transient Collections ;;

src/basilisp/lang/runtime.py

@@ -51,6 +51,7 @@
     IBlockingDeref,
     IDeref,
     ILookup,
+    IMapEntry,
     IPersistentCollection,
     IPersistentList,
     IPersistentMap,
@@ -59,6 +60,7 @@
     IPersistentVector,
     IReduce,
     ISeq,
+    ISeqable,
     ITransientAssociative,
     ITransientSet,
     ReduceFunction,
@@ -980,6 +982,40 @@ def pop_thread_bindings() -> None:
 T = TypeVar("T")
 
 
+@functools.singledispatch
+def to_set(s):
+    return lset.set(s)
+
+
+@to_set.register(type(None))
+def _to_set_none(_: None) -> lset.PersistentSet:
+    return lset.EMPTY
+
+
+@to_set.register(IPersistentMap)
+def _to_set_map(m: IPersistentMap) -> lset.PersistentSet:
+    if (s := m.seq()) is None:
+        return lset.EMPTY
+    return to_set(s)
+
+
+@functools.singledispatch
+def vector(v):
+    return vec.vector(v)
+
+
+@vector.register(type(None))
+def _vector_none(_: None) -> vec.PersistentVector:
+    return vec.EMPTY
+
+
+@vector.register(IPersistentMap)
+def _vector_map(m: IPersistentMap) -> vec.PersistentVector:
+    if (s := m.seq()) is None:
+        return vec.EMPTY
+    return vector(s)
+
+
 def keyword(name: Any, ns: Any = None) -> kw.Keyword:
     """Return a new keyword with runtime type checks for name and namespace."""
     if not isinstance(name, str):
@@ -1391,6 +1427,78 @@ def _update_iassociative(m: IAssociative, k, f, *args):
     return m.assoc(k, new_v)
 
 
+@functools.singledispatch
+def keys(o):
+    raise TypeError(f"Object of type {type(o)} cannot be coerced to a key sequence")
+
+
+@keys.register(type(None))
+def _keys_none(_: None) -> None:
+    return None
+
+
+@keys.register(collections.abc.Iterable)
+@keys.register(ISeqable)
+def _keys_iterable(o: Union[ISeqable, Iterable]) -> Optional[ISeq]:
+    return keys(to_seq(o))
+
+
+@keys.register(ISeq)
+def _keys_iseq(o: ISeq) -> Optional[ISeq]:
+    def _key_seq(s: ISeq) -> Optional[ISeq]:
+        if to_seq(s) is not None:
+            e = s.first
+            if not isinstance(e, IMapEntry):
+                raise TypeError(
+                    f"Object of type {type(e)} cannot be coerced to a map entry"
+                )
+            return lseq.Cons(e.key, lseq.LazySeq(lambda: _key_seq(s.rest)))
+        return None
+
+    return lseq.LazySeq(lambda: _key_seq(o))
+
+
+@keys.register(collections.abc.Mapping)
+def _keys_mapping(o: Mapping) -> Optional[ISeq]:
+    return to_seq(o.keys())
+
+
+@functools.singledispatch
+def vals(o):
+    raise TypeError(f"Object of type {type(o)} cannot be coerced to a value sequence")
+
+
+@vals.register(type(None))
+def _vals_none(_: None) -> None:
+    return None
+
+
+@keys.register(collections.abc.Iterable)
+@vals.register(ISeqable)
+def _vals_iterable(o: Union[ISeqable, Iterable]) -> Optional[ISeq]:
+    return vals(to_seq(o))
+
+
+@vals.register(ISeq)
+def _vals_iseq(o: ISeq) -> Optional[ISeq]:
+    def _val_seq(s: ISeq) -> Optional[ISeq]:
+        if to_seq(s) is not None:
+            e = s.first
+            if not isinstance(e, IMapEntry):
+                raise TypeError(
+                    f"Object of type {type(e)} cannot be coerced to a map entry"
+                )
+            return lseq.Cons(e.value, lseq.LazySeq(lambda: _val_seq(s.rest)))
+        return None
+
+    return lseq.LazySeq(lambda: _val_seq(o))
+
+
+@vals.register(collections.abc.Mapping)
+def _vals_mapping(o: Mapping) -> Optional[ISeq]:
+    return to_seq(o.values())
+
+
 @functools.singledispatch
 def conj(coll, *xs):
     """Conjoin xs to collection. New elements may be added in different positions

src/basilisp/lang/vector.py

@@ -300,11 +300,9 @@ def from_vec(v: Sequence[Union[K, V]]) -> "MapEntry[K, V]":
 
 
 def vector(
-    members: Optional[Iterable[T]], meta: Optional[IPersistentMap] = None
+    members: Iterable[T], meta: Optional[IPersistentMap] = None
 ) -> PersistentVector[T]:
     """Creates a new vector."""
-    if members is None:
-        members = []
     return PersistentVector(pvector(members), meta=meta)
 
 

tests/basilisp/core_test.py

@@ -1342,16 +1342,6 @@ def test_update_in(self):
             "u",
         )
 
-    def test_keys(self):
-        assert None is core.keys(lmap.map({}))
-        assert llist.l("a") == core.keys(lmap.map({"a": 1}))
-        assert lset.s("a", "b") == lset.set(core.keys(lmap.map({"a": 1, "b": 2})))
-
-    def test_vals(self):
-        assert None is core.vals(lmap.map({}))
-        assert llist.l(1) == core.vals(lmap.map({"a": 1}))
-        assert lset.s(1, 2) == lset.set(core.vals(lmap.map({"a": 1, "b": 2})))
-
     def test_select_keys(self):
         assert lmap.PersistentMap.empty() == core.select_keys(
             lmap.PersistentMap.empty(), vec.PersistentVector.empty()

tests/basilisp/test_core_fns.lpy

@@ -8,6 +8,35 @@
    [basilisp.set :as set]
    [basilisp.test :refer [deftest are is testing]]))
 
+(deftest vec-test
+  (is (= #{[:a 1] [:b 2]} (set (vec {:a 1 :b 2}))))
+  (is (= #{:a :b :c} (set (vec #{:a :b :c}))))
+
+  (are [res v] (= res (vec v))
+    []            nil
+    []            '()
+    []            []
+    []            #{}
+    []            {}
+    [:a :b :c]    [:a :b :c]
+    [:a :b :c]    '(:a :b :c)
+    ["a" "b" "c"] "abc"
+    [[:a 1]]      {:a 1}))
+
+(deftest set-test
+  (are [res v] (= res (set v))
+    #{}              nil
+    #{}              '()
+    #{}              []
+    #{}              #{}
+    #{}              {}
+    #{:a :b :c}      [:a :b :c]
+    #{:a :b :c}      '(:a :b :c)
+    #{:a :b :c}      #{:a :b :c}
+    #{"a" "b" "c"}   "abc"
+    #{[:a 1]}        {:a 1}
+    #{[:a 1] [:b 2]} {:a 1 :b 2}))
+
 (deftest keyword-test
   (testing "name only"
     (are [res name] (= res (keyword name))
@@ -431,16 +460,7 @@
   (is (= 1 (rand-nth [1])))
   (is (#{1 2} (rand-nth [1 2]))))
 
-(deftest vec-test
-  (are [expected x] (= expected (vec x))
-    []       []
-    []       ()
-    []       #{}
-    []       {}
-    []       nil
-    [1]      (list 1)
-    [1]      #{1}
-    [1 2]    (list 1 2)))
+
 
 (deftest subvec-test
   (is (= [] (subvec [] 0)))
@@ -1408,23 +1428,43 @@
     ^{:good :yes :great :also-yes} {:a "A" 'b 2} ^{:good :yes :great :also-yes} {:a "a" 'b 2}))
 
 (deftest keys-test
+  (is (thrown? python/TypeError (keys :abc)))
+  ;; use vec to actually realize the key sequence and trigger the error
+  (is (thrown? python/TypeError (vec (keys [:a :b :c]))))
+  (is (thrown? python/TypeError (vec (keys [[:a 1] [:b 2]]))))
+
   (are [expected m] (= expected (set (keys m)))
     #{}      {}
     #{}      nil
+    #{}    '()
+    #{}    []
+    #{}    #{}
     #{:a}    {:a 1}
     #{:a}    {:a nil}
     #{:a :b} {:a 1 :b 2}
+    #{:a :b} (seq {:a 1 :b 2})
+    #{:a :b} (vec {:a 1 :b 2})
     #{"a"}   {"a" 1}
     #{}      (python/dict {})
     #{:a}    (python/dict {:a 1})))
 
 (deftest vals-test
+  (is (thrown? python/TypeError (vals :abc)))
+  ;; use vec to actually realize the val sequence and trigger the error
+  (is (thrown? python/TypeError (vec (vals [:a :b :c]))))
+  (is (thrown? python/TypeError (vec (vals [[:a 1] [:b 2]]))))
+
   (are [expected m] (= expected (set (vals m)))
     #{}    {}
     #{}    nil
+    #{}    '()
+    #{}    []
+    #{}    #{}
     #{1}   {:a 1}
     #{nil} {:a nil}
     #{1 2} {:a 1 :b 2}
+    #{1 2} (seq {:a 1 :b 2})
+    #{1 2} (vec {:a 1 :b 2})
     #{1}   {"a" 1}
     #{}    (python/dict {})
     #{1}   (python/dict {:a 1})))