[140] Ensuring that lists and dictionaries are included in signatures (#141)

* work

* adding test

* removing old file

* remove old comment

Author: tjhunter

dds/_api.py

@@ -30,7 +30,7 @@
     FunctionInteractionsUtils,
     FunctionIndirectInteractionUtils,
 )
-from . import _options
+from ._config import get_option, extra_debug_option
 
 _Out = TypeVar("_Out")
 _In = TypeVar("_In")
@@ -192,7 +192,7 @@ def _eval(
 
     stages = _parse_stages(dds_stages)
 
-    extra_debug = dds_extra_debug or _options._dds_extra_debug
+    extra_debug = dds_extra_debug or get_option(extra_debug_option)
 
     if not _eval_ctx:
         # Not in an evaluation context, create one and introspect

dds/_config.py

@@ -105,7 +105,46 @@ def validate(self, v: Any) -> None:
 
 # Available options.
 #
+
+accept_list_option = Option(
+    key="accept_list",
+    doc=(
+        "Accepts lists as objects. If true, lists are then traversed and their content is included in the signature"
+        " (default true)"
+    ),
+    default=True,
+    types=(bool,),
+    check_func=(lambda v: True, "",),
+)
+
+accept_dict_option = Option(
+    key="accept_dict",
+    doc=(
+        "Accepts dictionaries as objects. If true, lists are then traversed and their content is included "
+        "in the signature"
+        " (default true)"
+    ),
+    default=True,
+    types=(bool,),
+    check_func=(lambda v: True, "",),
+)
+
+extra_debug_option = Option(
+    key="extra_debug",
+    doc=(
+        "Prints and evaluates extra debugging information. This information requires extra roundtrips to the "
+        "storage backend. It is disabled by default to assist with debugging, but it can be disabled if "
+        "I/O with the storage backend is an issue."
+    ),
+    default=True,
+    types=(bool,),
+    check_func=(lambda v: True, "",),
+)
+
 _options: List[Option] = [
+    extra_debug_option,
+    accept_list_option,
+    accept_dict_option,
     Option(
         key="hash.max_sequence_size",
         doc=(
@@ -154,7 +193,7 @@ def show_options():
     print(row_format.format("=" * 31, "=" * 14, "=" * 53))
 
 
-def get_option(key: str, default: Union[Any, None] = None) -> Any:
+def get_option(key: Union[Option, str], default: Union[Any, None] = None) -> Any:
     """
     Retrieves the value of the specified option.
 
@@ -173,6 +212,8 @@ def get_option(key: str, default: Union[Any, None] = None) -> Any:
     ------
     DDSException : if no such option exists and the default is not provided
     """
+    if isinstance(key, Option):
+        return get_option(key.key)
     _check_option(key)
     if default is None:
         default = _options_dict[key].default

dds/_options.py

@@ -23,6 +23,7 @@
 )
 from .structures import DDSException, CanonicalPath, LocalDepPath, DDSErrorCode
 from .structures_utils import LocalDepPathUtils, CanonicalPathUtils
+from ._config import get_option, accept_list_option, accept_dict_option
 
 _logger = logging.getLogger(__name__)
 
@@ -55,6 +56,11 @@ def _is_authorized_type(tpe: Type[Any], gctx: EvalMainContext) -> bool:
         return True
     if tpe in (int, float, str, bytes, PurePosixPath, FunctionType, ModuleType):
         return True
+    # Some specific structural types are more complex and can be user-controlled.
+    if get_option(accept_list_option) and tpe in (list,):
+        return True
+    if get_option(accept_dict_option) and tpe in (dict,):
+        return True
     if issubclass(tpe, object):
         mod = inspect.getmodule(tpe)
         if mod is None:
@@ -90,9 +96,11 @@ def retrieve_object(
         obj_key = (local_path, mod_path)
 
         if obj_key in gctx.cached_objects:
-            # _logger.debug(f"retrieve_object: found in cache: obj_key: {obj_key}")
+            if debug:
+                _logger.debug(f"retrieve_object: found in cache: obj_key: {obj_key}")
             return gctx.cached_objects[obj_key]
-        # _logger.debug(f"retrieve_object: not found in cache: obj_key: {obj_key}")
+        if debug:
+            _logger.debug(f"retrieve_object: not found in cache: obj_key: {obj_key}")
 
         fname = local_path.parts[0]
         sub_path = LocalDepPathUtils.tail(local_path)
@@ -112,21 +120,19 @@ def retrieve_object(
                 # Looking into the globals (only if the scope is currently __main__ or __global__)
                 mod_path = _mod_path(context_mod)
                 if CanonicalPathUtils.head(mod_path) not in ("__main__", "__global__"):
-                    # _logger.debug(
-                    #     f"Could not load name %s and not in global context (%s), skipping ",
-                    #     fname,
-                    #     mod_path,
-                    # )
+                    if debug:
+                        _logger.debug(
+                            f"Could not load name %s and not in global context (%s), skipping ",
+                            fname,
+                            mod_path,
+                        )
                     return None
                 else:
-                    # _logger.debug(
-                    #     f"Could not load name %s, looking into the globals (mod_path: %s, %s)",
-                    #     fname,
-                    #     mod_path,
-                    #     mod_path.get(0),
-                    # )
                     pass
-                # _logger.debug(f"Could not load name {fname}, looking into the globals")
+                if debug:
+                    _logger.debug(
+                        f"Could not load name {fname}, looking into the globals"
+                    )
                 if fname in gctx.start_globals:
                     # _logger.debug(f"Found {fname} in start_globals")
                     obj = gctx.start_globals[fname]
@@ -151,10 +157,11 @@ def retrieve_object(
                             ["__global__"] + [str(x) for x in local_path.parts]
                         )
                     if not gctx.is_authorized_path(obj_path):
-                        # _logger.debug(
-                        #     f"Object[start_globals] {fname} of type {type(obj)} is not authorized (path),"
-                        #     f" dropping path {obj_path}"
-                        # )
+                        if debug:
+                            _logger.debug(
+                                f"Object[start_globals] {fname} of type {type(obj)} is not authorized (path),"
+                                f" dropping path {obj_path}"
+                            )
                         res = ExternalObject(obj_path)
                         gctx.cached_objects[obj_key] = res
                         return res
@@ -171,21 +178,24 @@ def retrieve_object(
                             str,
                         ),
                     ):
-                        # _logger.debug(
-                        #     f"Object[start_globals] {fname} ({type(obj)}) of path {obj_path} is authorized,"
-                        # )
+                        if debug:
+                            _logger.debug(
+                                f"Object[start_globals] {fname} ({type(obj)}) of path {obj_path} is authorized,"
+                            )
                         res = AuthorizedObject(obj, obj_path)
                         gctx.cached_objects[obj_key] = res
                         return res
                     else:
-                        # _logger.debug(
-                        #     f"Object[start_globals] {fname} of type {type(obj)} is noft authorized (type), dropping path {obj_path}"
-                        # )
+                        if debug:
+                            _logger.debug(
+                                f"Object[start_globals] {fname} of type {type(obj)} is noft authorized (type), dropping path {obj_path}"
+                            )
                         res = ExternalObject(obj_path)
                         gctx.cached_objects[obj_key] = res
                         return res
                 else:
-                    # _logger.debug(f"{fname} not found in start_globals")
+                    if debug:
+                        _logger.debug(f"{fname} not found in start_globals")
                     gctx.cached_objects[obj_key] = None
                     return None
             res = cls._retrieve_object_rec(sub_path, loaded_mod, gctx)

dds/_retrieve_objects.py

@@ -81,7 +81,8 @@ def check_len(x: Any) -> None:
                 f"Object of type {type(x)} is a sequence of length {len(x)}. "
                 f"Only sequences of length less than {max_sequence_size} are supported. "
                 "This behaviour can be adjusted with the 'hash.max_sequence_size' option."
-                f" Path hint: <{current_path()}>"
+                f" Path hint: <{current_path()}>",
+                DDSErrorCode.SEQUENCE_TOO_LONG,
             )
 
     def _dds_hash(elt: Any, path_item: Union[int, str, None]) -> PyHash:
@@ -92,6 +93,14 @@ def _dds_hash(elt: Any, path_item: Union[int, str, None]) -> PyHash:
             trace.pop()
         return res
 
+    def _hash_dict_tuple(k: Any, v: Any) -> str:
+        # Supposing for now that any dictionary key is well-behaved with respect to being converted to a string.
+        if isinstance(k, str):
+            n = k
+        else:
+            n = str(k)
+        return _dds_hash(k, None) + "|" + _dds_hash(v, n)
+
     def _dds_hash0(elt: Any) -> PyHash:
         if elt is None:
             # TODO: this is not robust to adversarial changes.
@@ -126,16 +135,16 @@ def _dds_hash0(elt: Any) -> PyHash:
             # majority of python interpreters out there).
             # Not going to check for obscure corner cases for now.
             check_len(elt)
-            return _dds_hash(
-                [name + "|" + _dds_hash(v, name) for (name, v) in elt.items()], None
-            )
+            return _dds_hash([_hash_dict_tuple(k, v) for (k, v) in elt.items()], None)
         if dataclasses.is_dataclass(elt):
             names: List[str] = [f.name for f in dataclasses.fields(elt)]
             # TODO: this is not entirely accurate. The error message will show a 'list' type, but it is actually
             # a dataclass.
             check_len(names)
             vals = [_dds_hash(getattr(elt, n), n) for n in names]
-            return _dds_hash([name + "|" + h for (name, h) in zip(names, vals)], None)
+            return _dds_hash(
+                [_hash_dict_tuple(name, h) for (name, h) in zip(names, vals)], None
+            )
         if isinstance(
             elt,
             (

dds/fun_args.py

@@ -436,25 +436,29 @@ def __init__(
 
     def visit_Name(self, node: ast.Name, debug: bool = False) -> Any:
         local_dep_path = LocalDepPath(PurePosixPath(node.id))
-        # _logger.debug(
-        #     "ExternalVarsVisitor:visit_Name: id: %s local_dep_path:%s",
-        #     node.id,
-        #     local_dep_path,
-        # )
+        if debug:
+            _logger.debug(
+                "ExternalVarsVisitor:visit_Name: id: %s local_dep_path:%s",
+                node.id,
+                local_dep_path,
+            )
         if not isinstance(node.ctx, ast.Load):
-            # _logger.debug(
-            #     "ExternalVarsVisitor:visit_Name: id: %s skipping ctx: %s",
-            #     node.id,
-            #     node.ctx,
-            # )
+            if debug:
+                _logger.debug(
+                    "ExternalVarsVisitor:visit_Name: id: %s skipping ctx: %s",
+                    node.id,
+                    node.ctx,
+                )
             return
         # If it is a var that is already part of the function, do not introspect
         if len(local_dep_path.parts) == 1:
             v = str(local_dep_path)
             if v in self._local_vars:
-                # _logger.debug(
-                #     "ExternalVarsVisitor:visit_Name: id: %s skipping, in vars", node.id
-                # )
+                if debug:
+                    _logger.debug(
+                        "ExternalVarsVisitor:visit_Name: id: %s skipping, in vars",
+                        node.id,
+                    )
                 return
         if local_dep_path in self.vars or local_dep_path in self._rejected_paths:
             return
@@ -658,7 +662,7 @@ def inspect_fun(
             vdeps.visit(n)
         ext_deps = sorted(vdeps.vars.values(), key=lambda ed: ed.local_path)
         if debug:
-            _logger.debug(f"inspect_fun: ext_deps: %s", ext_deps)
+            _logger.debug("inspect_fun: ext_deps: %s", ext_deps)
 
         # The variables that are hashable: authorized variables outside of the function
         sig_variables: List[Tuple[LocalDepPath, PyHash]] = [

dds/introspect.py

@@ -58,16 +58,19 @@ class DDSErrorCode(IntEnum):
     ARG_IN_DATA_FUNCTION = 15
     OVERLAPPING_PATH = 16
     UNKNOWN_OPTION = 17
+    SEQUENCE_TOO_LONG = 18
 
 
 class DDSException(BaseException):
     """
     The base exception for all the exceptions generated in DDS.
     """
 
+    error_code: Optional[DDSErrorCode]
+
     def __init__(self, message: str, error_code: Optional[DDSErrorCode] = None):
         super(DDSException, self).__init__(message)
-        self.error_code: Optional[DDSErrorCode] = error_code
+        self.error_code = error_code
 
 
 class EvalContext(NamedTuple):

dds/structures.py

@@ -219,7 +219,9 @@ def fun_9():
 
 
 @pytest.mark.usefixtures("cleandir")
-def test_9():
+def test_gh140_biglists():
     """ Using big objects throws an error """
     # TODO: more comprehensive test on lists. They are still seen as external dependencies
-    assert dds.eval(fun_9) == test_9_len
+    with pytest.raises(dds.DDSException) as e:
+        dds.eval(fun_9)
+    assert e.value.error_code == dds.structures.DDSErrorCode.SEQUENCE_TOO_LONG

dds_tests/test_refs.py

@@ -0,0 +1,51 @@
+import dds
+import pytest
+from .utils import cleandir, Counter
+
+_ = cleandir
+
+_c = Counter()
+
+l = [1]
+
+
+@dds.data_function("/p")
+def f1():
+    _c.increment()
+    return len(l)
+
+
+@pytest.mark.usefixtures("cleandir")
+def test_gh140_list():
+    global l
+    _c.reset()
+    f1()
+    assert _c.value == 1
+    l[0] = 2
+    f1()
+    assert _c.value == 2
+    l[0] = 1
+    f1()
+    assert _c.value == 2
+
+
+d = {0: 1}
+
+
+@dds.data_function("/p")
+def f2():
+    _c.increment()
+    return len(d)
+
+
+@pytest.mark.usefixtures("cleandir")
+def test_gh140_dict():
+    _c.reset()
+    f2()
+    assert _c.value == 1
+    d[0] = 2
+    f2()
+    assert _c.value == 2
+    d[0] = 1
+    f2()
+    assert _c.value == 2

dds_tests/test_structures.py

@@ -64,6 +64,9 @@ def __init__(self):
     def increment(self):
         self.value += 1
 
+    def reset(self):
+        self.value = 0
+
 
 def unreachable():
     # Will trigger a failure in the parsing