*print-namespace-maps* support (#883)

Hi,

could you please review draft support for the *print-namespace-maps*
dynamic variable. It resolves #882.

I implemented the support at the lowest `obj.py` level, though this
required to bring in the `keyword.py` and `symbol.py` modules resulting
to cyclic dependencies. Not sure how else to implement it, so any other
suggestions are most welcome.

I've also blindly followed the example of *print-dup* on how I ended
setup the variable in the cli/runtime/object .py files.

In addition, I've used a set to compare the printed output of the map
result with multiple k/v, exhaustively covering the possible orderings,
is there perhaps a better simpler way to do this? This currently is
prune to user errors.

Thanks

---------

Co-authored-by: ikappaki <[email protected]>

Author: ikappaki

CHANGELOG.md

@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
  * Added `merge-with` core function (#860)
  * Added `fnext` core function (#879)
  * Added `INamed` interface for Keywords and Symbols (#884)
+ * Added `*print-namespace-maps*` dynamic var support (#882)
 
 ### Changed
  * Cause exceptions arising from compilation issues during macroexpansion will no longer be nested for each level of macroexpansion (#852)

src/basilisp/cli.py

@@ -422,6 +422,7 @@ def repl(
                     runtime.PRINT_READABLY_VAR_NAME,
                     runtime.PRINT_LEVEL_VAR_NAME,
                     runtime.PRINT_META_VAR_NAME,
+                    runtime.PRINT_NAMESPACE_MAPS_VAR_NAME,
                 ],
             )
         }

src/basilisp/lang/map.py

@@ -1,5 +1,16 @@
 from builtins import map as pymap
-from typing import Callable, Iterable, Mapping, Optional, Tuple, TypeVar, Union, cast
+from itertools import islice
+from typing import (
+    Any,
+    Callable,
+    Iterable,
+    Mapping,
+    Optional,
+    Tuple,
+    TypeVar,
+    Union,
+    cast,
+)
 
 from immutables import Map as _Map
 from immutables import MapMutation
@@ -9,14 +20,21 @@
     IEvolveableCollection,
     ILispObject,
     IMapEntry,
+    INamed,
     IPersistentMap,
     IPersistentVector,
     ISeq,
     ITransientMap,
     IWithMeta,
 )
-from basilisp.lang.obj import PrintSettings
-from basilisp.lang.obj import map_lrepr as _map_lrepr
+from basilisp.lang.obj import (
+    PRINT_SEPARATOR,
+    SURPASSED_PRINT_LENGTH,
+    SURPASSED_PRINT_LEVEL,
+    PrintSettings,
+    lrepr,
+    process_lrepr_kwargs,
+)
 from basilisp.lang.seq import sequence
 from basilisp.lang.vector import MapEntry
 from basilisp.util import partition
@@ -107,6 +125,91 @@ def to_persistent(self) -> "PersistentMap[K, V]":
         return PersistentMap(self._inner.finish())
 
 
+def map_lrepr(  # pylint: disable=too-many-locals
+    entries: Callable[[], Iterable[Tuple[Any, Any]]],
+    start: str,
+    end: str,
+    meta: Optional[IPersistentMap] = None,
+    **kwargs: Unpack[PrintSettings],
+) -> str:
+    """Produce a Lisp representation of an associative collection, bookended
+    with the start and end string supplied. The entries argument must be a
+    callable which will produce tuples of key-value pairs.
+
+    If the keyword argument print_namespace_maps is True and all keys
+    share the same namespace, then print the namespace of the keys at
+    the beginning of the map instead of beside the keys.
+
+    The keyword arguments will be passed along to lrepr for the sequence
+    elements.
+
+    """
+    print_level = kwargs["print_level"]
+    if isinstance(print_level, int) and print_level < 1:
+        return SURPASSED_PRINT_LEVEL
+
+    kwargs = process_lrepr_kwargs(**kwargs)
+
+    def check_same_ns():
+        """Check whether all keys in entries belong to the same
+        namespace. If they do, return the namespace name; otherwise,
+        return None.
+        """
+        nses = set()
+        for k, _ in entries():
+            if isinstance(k, INamed):
+                nses.add(k.ns)
+            else:
+                nses.add(None)
+            if len(nses) > 1:
+                break
+        return next(iter(nses)) if len(nses) == 1 else None
+
+    ns_name_shared = check_same_ns() if kwargs["print_namespace_maps"] else None
+
+    entries_updated = entries
+    if ns_name_shared:
+
+        def entries_ns_remove():
+            for k, v in entries():
+                yield (k.with_name(k.name), v)
+
+        entries_updated = entries_ns_remove
+
+    kw_items = kwargs.copy()
+    kw_items["human_readable"] = False
+
+    def entry_reprs():
+        for k, v in entries_updated():
+            yield f"{lrepr(k, **kw_items)} {lrepr(v, **kw_items)}"
+
+    trailer = []
+    print_dup = kwargs["print_dup"]
+    print_length = kwargs["print_length"]
+    if not print_dup and isinstance(print_length, int):
+        items = list(islice(entry_reprs(), print_length + 1))
+        if len(items) > print_length:
+            items.pop()
+            trailer.append(SURPASSED_PRINT_LENGTH)
+    else:
+        items = list(entry_reprs())
+
+    seq_lrepr = PRINT_SEPARATOR.join(items + trailer)
+
+    ns_prefix = ("#:" + ns_name_shared) if ns_name_shared else ""
+    if kwargs["print_meta"] and meta:
+        kwargs_meta = kwargs
+        kwargs_meta["print_level"] = None
+        return f"^{lrepr(meta,**kwargs_meta)} {ns_prefix}{start}{seq_lrepr}{end}"
+
+    return f"{ns_prefix}{start}{seq_lrepr}{end}"
+
+
+@lrepr.register(dict)
+def _lrepr_py_dict(o: dict, **kwargs: Unpack[PrintSettings]) -> str:
+    return f"#py {map_lrepr(o.items, '{', '}', **kwargs)}"
+
+
 class PersistentMap(
     IPersistentMap[K, V], IEvolveableCollection[TransientMap], ILispObject, IWithMeta
 ):
@@ -163,8 +266,12 @@ def __len__(self):
         return len(self._inner)
 
     def _lrepr(self, **kwargs: Unpack[PrintSettings]):
-        return _map_lrepr(
-            self._inner.items, start="{", end="}", meta=self._meta, **kwargs
+        return map_lrepr(
+            self._inner.items,
+            start="{",
+            end="}",
+            meta=self._meta,
+            **kwargs,
         )
 
     @property

src/basilisp/lang/obj.py

@@ -8,7 +8,7 @@
 from functools import singledispatch
 from itertools import islice
 from pathlib import Path
-from typing import Any, Callable, Iterable, Pattern, Tuple, Union, cast
+from typing import Any, Iterable, Pattern, Union, cast
 
 from typing_extensions import TypedDict, Unpack
 
@@ -21,6 +21,7 @@
 PRINT_LENGTH: PrintCountSetting = 50
 PRINT_LEVEL: PrintCountSetting = None
 PRINT_META = False
+PRINT_NAMESPACE_MAPS = False
 PRINT_READABLY = True
 PRINT_SEPARATOR = " "
 
@@ -31,6 +32,7 @@ class PrintSettings(TypedDict, total=False):
     print_length: PrintCountSetting
     print_level: PrintCountSetting
     print_meta: bool
+    print_namespace_maps: bool
     print_readably: bool
 
 
@@ -41,7 +43,7 @@ def _dec_print_level(lvl: PrintCountSetting) -> PrintCountSetting:
     return lvl
 
 
-def _process_kwargs(**kwargs: Unpack[PrintSettings]) -> PrintSettings:
+def process_lrepr_kwargs(**kwargs: Unpack[PrintSettings]) -> PrintSettings:
     """Process keyword arguments, decreasing the print-level. Should be called
     after examining the print level for the current level."""
     return cast(
@@ -78,52 +80,6 @@ def lrepr(self, **kwargs: Unpack[PrintSettings]) -> str:
         return lrepr(self, **kwargs)
 
 
-def map_lrepr(
-    entries: Callable[[], Iterable[Tuple[Any, Any]]],
-    start: str,
-    end: str,
-    meta=None,
-    **kwargs: Unpack[PrintSettings],
-) -> str:
-    """Produce a Lisp representation of an associative collection, bookended
-    with the start and end string supplied. The entries argument must be a
-    callable which will produce tuples of key-value pairs.
-
-    The keyword arguments will be passed along to lrepr for the sequence
-    elements."""
-    print_level = kwargs["print_level"]
-    if isinstance(print_level, int) and print_level < 1:
-        return SURPASSED_PRINT_LEVEL
-
-    kwargs = _process_kwargs(**kwargs)
-
-    kw_items = kwargs.copy()
-    kw_items["human_readable"] = False
-
-    def entry_reprs():
-        for k, v in entries():
-            yield f"{lrepr(k, **kw_items)} {lrepr(v, **kw_items)}"
-
-    trailer = []
-    print_dup = kwargs["print_dup"]
-    print_length = kwargs["print_length"]
-    if not print_dup and isinstance(print_length, int):
-        items = list(islice(entry_reprs(), print_length + 1))
-        if len(items) > print_length:
-            items.pop()
-            trailer.append(SURPASSED_PRINT_LENGTH)
-    else:
-        items = list(entry_reprs())
-
-    seq_lrepr = PRINT_SEPARATOR.join(items + trailer)
-
-    print_meta = kwargs["print_meta"]
-    if print_meta and meta:
-        return f"^{lrepr(meta, **kwargs)} {start}{seq_lrepr}{end}"
-
-    return f"{start}{seq_lrepr}{end}"
-
-
 def seq_lrepr(
     iterable: Iterable[Any],
     start: str,
@@ -138,7 +94,7 @@ def seq_lrepr(
     if isinstance(print_level, int) and print_level < 1:
         return SURPASSED_PRINT_LEVEL
 
-    kwargs = _process_kwargs(**kwargs)
+    kwargs = process_lrepr_kwargs(**kwargs)
 
     trailer = []
     print_dup = kwargs["print_dup"]
@@ -172,6 +128,7 @@ def lrepr(  # pylint: disable=too-many-arguments
     print_length: PrintCountSetting = PRINT_LENGTH,
     print_level: PrintCountSetting = PRINT_LEVEL,
     print_meta: bool = PRINT_META,
+    print_namespace_maps: bool = PRINT_NAMESPACE_MAPS,
     print_readably: bool = PRINT_READABLY,
 ) -> str:
     """Return a string representation of a Lisp object.
@@ -185,6 +142,10 @@ def lrepr(  # pylint: disable=too-many-arguments
                     or no limit if bound to a logical falsey value (default: 50)
     - print_level: the depth of the object graph to print, starting with 0, or
                    no limit if bound to a logical falsey value (default: nil)
+    - print_namespace_maps: if logical true, and the object is a map consisting
+                            with keys belonging to the same namespace, print the
+                            namespace at the beginning of the map instead of
+                            beside the keys (default: false)
     - print_meta: if logical true, print objects meta in a way that can be
                   read back by the reader (default: false)
     - print_readably: if logical false, print strings and characters with
@@ -206,6 +167,7 @@ def _lrepr_lisp_obj(  # pylint: disable=too-many-arguments
     print_length: PrintCountSetting = PRINT_LENGTH,
     print_level: PrintCountSetting = PRINT_LEVEL,
     print_meta: bool = PRINT_META,
+    print_namespace_maps: bool = PRINT_NAMESPACE_MAPS,
     print_readably: bool = PRINT_READABLY,
 ) -> str:  # pragma: no cover
     return o._lrepr(
@@ -214,6 +176,7 @@ def _lrepr_lisp_obj(  # pylint: disable=too-many-arguments
         print_length=print_length,
         print_level=print_level,
         print_meta=print_meta,
+        print_namespace_maps=print_namespace_maps,
         print_readably=print_readably,
     )
 
@@ -250,11 +213,6 @@ def _lrepr_py_list(o: list, **kwargs: Unpack[PrintSettings]) -> str:
     return f"#py {seq_lrepr(o, '[', ']', **kwargs)}"
 
 
-@lrepr.register(dict)
-def _lrepr_py_dict(o: dict, **kwargs: Unpack[PrintSettings]) -> str:
-    return f"#py {map_lrepr(o.items, '{', '}', **kwargs)}"
-
-
 @lrepr.register(set)
 def _lrepr_py_set(o: set, **kwargs: Unpack[PrintSettings]) -> str:
     return f"#py {seq_lrepr(o, '#{', '}', **kwargs)}"

src/basilisp/lang/runtime.py

@@ -91,6 +91,7 @@
 PRINT_LENGTH_VAR_NAME = "*print-length*"
 PRINT_LEVEL_VAR_NAME = "*print-level*"
 PRINT_META_VAR_NAME = "*print-meta*"
+PRINT_NAMESPACE_MAPS_VAR_NAME = "*print-namespace-maps*"
 PRINT_READABLY_VAR_NAME = "*print-readably*"
 PYTHON_VERSION_VAR_NAME = "*python-version*"
 BASILISP_VERSION_VAR_NAME = "*basilisp-version*"
@@ -1636,6 +1637,7 @@ def lrepr(o, human_readable: bool = False) -> str:
             sym.symbol(PRINT_LEVEL_VAR_NAME)
         ).value,
         print_meta=core_ns.find(sym.symbol(PRINT_META_VAR_NAME)).value,  # type: ignore
+        print_namespace_maps=core_ns.find(sym.symbol(PRINT_NAMESPACE_MAPS_VAR_NAME)).value,  # type: ignore
         print_readably=core_ns.find(  # type: ignore
             sym.symbol(PRINT_READABLY_VAR_NAME)
         ).value,
@@ -2191,6 +2193,21 @@ def in_ns(s: sym.Symbol):
     Var.intern(
         CORE_NS_SYM, sym.symbol(PRINT_META_VAR_NAME), lobj.PRINT_META, dynamic=True
     )
+    Var.intern(
+        CORE_NS_SYM,
+        sym.symbol(PRINT_NAMESPACE_MAPS_VAR_NAME),
+        lobj.PRINT_NAMESPACE_MAPS,
+        dynamic=True,
+        meta=lmap.map(
+            {
+                _DOC_META_KEY: (
+                    "Indicates to print the namespace of keys in a map belonging to the same"
+                    " namespace, at the beginning of the map instead of beside the keys."
+                    " Defaults to false."
+                )
+            }
+        ),
+    )
     Var.intern(
         CORE_NS_SYM,
         sym.symbol(PRINT_READABLY_VAR_NAME),

tests/basilisp/test_core_fns.lpy

@@ -2156,7 +2156,42 @@
 
 (deftest pr-test
   (testing "is dynamic"
-    (is (= '(1) (binding [pr (fn [& more] more)] (pr 1))))))
+    (is (= '(1) (binding [pr (fn [& more] more)] (pr 1)))))
+
+  (testing "with *print-namespace-maps*"
+    (is (= "#py {:a/x 5}" (binding [*print-namespace-maps* false] (pr-str #py {:a/x 5}))))
+    (is (= "#py #:a{:x 5}" (binding [*print-namespace-maps* true] (pr-str #py {:a/x 5}))))
+
+    (are [res m b] (contains? res (binding [*print-namespace-maps* b] (pr-str m)))
+      #{"{:a/x 5}"} {:a/x 5} false
+      #{"#:a{:x 5}"} {:a/x 5} true
+      #{"{:x 5}"} {:x 5} true
+      #{"{1 2}"} {1 2} false
+      #{"{1 2}"} {1 2} true
+      #{"{:a/y 6 :a/x 5}" "{:a/x 5 :a/y 6}"} {:a/x 5 :a/y 6} false
+      #{"#:a{:y 6 :x 5}" "#:a{:x 5 :y 6}"} {:a/x 5 :a/y 6} true
+      #{"{6 6 :a/x 5}" "{:a/x 5 6 6}"} {:a/x 5 6 6} false
+      #{"{6 6 :a/x 5}" "{:a/x 5 6 6}"} {:a/x 5 6 6} true
+      #{"{:a/x 5 :b/y 6}" "{:b/y 6 :a/x 5}"} {:a/x 5 :b/y 6} true
+      #{"#:a{y 6 x 5}" "#:a{x 5 y 6}"} {'a/x 5 'a/y 6} true
+      #{"#:a{y 6 :x 5}" "#:a{:x 5 y 6}"} {:a/x 5 'a/y 6} true
+      #{"{y 6 :a/x 5}" "{:a/x 5 y 6}"} {:a/x 5 'y 6} true)
+
+    (are [res m pnm pm] (= res (binding [*print-namespace-maps* pnm
+                                         *print-meta*           pm]
+                                 (pr-str m)))
+      "^{:m 1} {:a/x 5}"     ^{:m 1} {:a/x 5}   false true
+      "^{:m 1} #:a{:x 5}"    ^{:m 1} {:a/x 5}   true true
+      "^#:l{:m 1} #:a{:x 5}" ^{:l/m 1} {:a/x 5} true true)
+
+    (are [res m pl] (= res (binding [*print-namespace-maps* true
+                                     *print-meta*           true
+                                     *print-level* pl]
+                             (pr-str m)))
+      "#"                                ^{:m 1} {:a/x 5} 0
+      "^{:m 1} #:a{:x 5}"                ^{:m 1} {:a/x 5} 1
+      "^{:m 1} #:a{:x #}"                ^{:m 1} {:a/x ^{:x 22} {:b 36}} 1
+      "^{:m 1} #:a{:x ^{:x 22} {:b 36}}" ^{:m 1} {:a/x ^{:x 22} {:b 36}} 2)))
 
 (defn- bio-write
   "Helper fn to write the ``strings`` to the ByteIO buffer ``bio``