fix: disallow_overwrites no-op and cache_func_outputs default/kwargs bugs

dol/caching.py

@@ -2623,14 +2623,43 @@ class HashableDict(HashableMixin, dict):
 
 # NOTE: cache uses (func, args, kwargs). Don't want to make more complex with a bind cast to (func, kwargs) only
 def cache_func_outputs(cache=HashableDict):
-    """Decorator factory intended to cache a function's outputs in ``cache``, keyed by ``(func, args, kwargs)``;
-    only positional-argument calls with an explicitly given ``cache`` actually hit the cache."""
+    """Decorator factory that caches a function's outputs in ``cache``, keyed by
+    ``(func, args, kwargs)``.
+
+    :param cache: A cache instance (with ``__contains__``/``__getitem__``/
+        ``__setitem__``), or a zero-arg factory/class (e.g. the default,
+        ``HashableDict``) used to make one.
+
+    Note: like ``args``, every ``kwargs`` value must be hashable (they're part of
+    the cache key). Passing an unhashable value (e.g. a ``list``) raises
+    ``TypeError`` rather than silently skipping the cache.
+
+    >>> @cache_func_outputs()
+    ... def f(x, y=2):
+    ...     print(f"computing f({x}, {y})")
+    ...     return x + y
+    >>> f(1)
+    computing f(1, 2)
+    3
+    >>> f(1)  # cached: no "computing" print
+    3
+    >>> f(1, y=3)  # different kwargs: not a cache hit
+    computing f(1, 3)
+    4
+    >>> f(1, y=3)  # this one is now cached too
+    4
+    """
+    if isinstance(cache, type):
+        cache = cache()  # instantiate a cache class/factory
     cache = get_cache(cache)
 
     def cache_method_decorator(func):
         @wraps(func)
         def _func(*args, **kwargs):
-            k = (func, args, HashableDict(kwargs))
+            # Use a hashable-by-value key for kwargs (a HashableDict instance is
+            # hashable by id, so a *fresh* one would never compare equal to another
+            # with the same contents -- see i2mint/dol#101).
+            k = (func, args, tuple(sorted(kwargs.items())))
             if k not in cache:
                 val = func(*args, **kwargs)
                 cache[k] = val  # cache it

dol/trans.py

@@ -684,20 +684,66 @@ def _ipython_key_completions_(self):
 
 
 def disallow_overwrites(store, *, error_msg=None, disable_deletes=True):
-    """Intended to make a store class's ``__setitem__`` raise ``OverWritesNotAllowedError`` on existing keys;
-    currently a no-op that returns ``None`` (the override is never attached). Use ``OverWritesNotAllowedMixin``."""
+    """Return a subclass of ``store`` whose ``__setitem__`` raises
+    ``OverWritesNotAllowedError`` on existing keys (``store`` itself is left
+    untouched).
+
+    :param store: The store class to wrap (must be a type).
+    :param error_msg: Custom error message; ``{}`` (or ``{k}``) in it is filled
+        in with the offending key via ``.format``. Defaults to a generic message.
+    :param disable_deletes: If ``True`` (the default), also disable
+        ``__delitem__`` (raising the same error) -- since deleting a key and
+        rewriting it would otherwise be a way around the overwrite guard.
+    :return: A new subclass of ``store`` with the guard(s) attached.
+
+    >>> class D(dict): ...
+    >>> ND = disallow_overwrites(D)
+    >>> d = ND(a=1)
+    >>> d['b'] = 2
+    >>> d['a'] = 1
+    Traceback (most recent call last):
+      ...
+    dol.errors.OverWritesNotAllowedError: key a already exists and cannot be overwritten...
+    >>> del d['a']
+    Traceback (most recent call last):
+      ...
+    dol.errors.OverWritesNotAllowedError: delete of key a is not allowed
+
+    With ``disable_deletes=False``, deletes are left alone:
+
+    >>> ND2 = disallow_overwrites(D, disable_deletes=False)
+    >>> d2 = ND2(a=1)
+    >>> del d2['a']  # no error
+    >>> d2['a'] = 2  # no error either, since 'a' was deleted first
+    """
     assert isinstance(store, type), "store needs to be a type"
+    if error_msg is None:
+        error_msg = (
+            "key {} already exists and cannot be overwritten. "
+            "If you really want to write to that key, delete it before writing"
+        )
+
+    namespace = {}
+
     if hasattr(store, "__setitem__"):
 
         def __setitem__(self, k, v):
             if k in self:
-                raise OverWritesNotAllowedError(
-                    "key {} already exists and cannot be overwritten. "
-                    "If you really want to write to that key, delete it before writing".format(
-                        k
-                    )
-                )
-            return super(type(self), self).__setitem__(k, v)
+                raise OverWritesNotAllowedError(error_msg.format(k, k=k))
+            return super(NoOverwritesStore, self).__setitem__(k, v)
+
+        namespace["__setitem__"] = __setitem__
+
+    if disable_deletes and hasattr(store, "__delitem__"):
+
+        def __delitem__(self, k):
+            raise OverWritesNotAllowedError(f"delete of key {k} is not allowed")
+
+        namespace["__delitem__"] = __delitem__
+
+    NoOverwritesStore = type(store.__name__, (store,), namespace)
+    copy_attrs(NoOverwritesStore, store, ("__name__", "__qualname__", "__module__"))
+    return NoOverwritesStore
 
 
 class OverWritesNotAllowedMixin: