Merge pull request #395 from Crunch-io/get-rid-of-deepcopy-187090869

Remove deepcopy from the codebase

src/cr/cube/dimension.py

@@ -2,7 +2,6 @@
 
 """Provides the Dimension class."""
 
-import copy
 from collections.abc import Sequence
 from functools import partial
 from typing import Callable, Dict, List, Optional, Tuple, Union
@@ -50,6 +49,16 @@ def _formatter(dimension_type, typedef, out_format) -> Union[Callable, partial]:
     return formatter
 
 
+def _build_element_id(element_dict, dimension_type) -> Union[int, str]:
+    "Returns the element id value according to the dimension tpye."
+    if dimension_type == DT.DATETIME and "datetime_value" in element_dict:
+        return element_dict["datetime_value"]
+    elif dimension_type in DT.ARRAY_TYPES and "subvar_alias" in element_dict:
+        return element_dict["subvar_alias"]
+    else:
+        return element_dict["id"]
+
+
 class Dimensions(tuple):
     """Collection containing every dimension defined in cube response."""
 
@@ -523,12 +532,12 @@ def from_typedef(
         elements = []
         for idx, element_dict in enumerate(element_defs):
             # --- convert to string for categorical ids
-            element_id = element_dict["id"]
+            element_id = _build_element_id(element_dict, dimension_type)
             xforms = _ElementTransforms(
                 all_xforms.get(element_id, all_xforms.get(str(element_id), {}))
             )
             formatter = _formatter(dimension_type, typedef, element_data_format)
-            element = Element(element_dict, idx, xforms, formatter)
+            element = Element(element_dict, idx, xforms, formatter, dimension_type)
             elements.append(element)
 
         return cls(elements)
@@ -553,7 +562,8 @@ def _hidden_transforms(cls, element_defs, insertions) -> Dict:
         # --- however, the hide-transform must be identified by element-id, so we need a
         # --- mapping of insertion-name to element-id
         element_id_from_name = {
-            element["value"]["id"]: element["id"] for element in element_defs
+            element["value"]["id"]: element.get("subvar_alias") or element["id"]
+            for element in element_defs
         }
 
         return {
@@ -629,33 +639,39 @@ def __init__(self, dimension_type, dimension_dict, dimension_transforms_dict):
 
     @lazyproperty
     def shimmed_dimension_dict(self) -> Dict:
-        """Copy of dimension dictionary with shimmed `element_id`s
+        """Augmented dimension dictionary with shimmed `element_id`s
 
         We want to move to a world where elements on a subvariables dimension are
         identified by their alias, but right now the "element_id" from zz9 is
         an index for subvariables.
         """
-        shim = copy.deepcopy(self._dimension_dict)
+        shim = self._dimension_dict
 
         # --- array variables are identified by thier aliases
         if self.dimension_type in DT.ARRAY_TYPES:
-            # --- Replace element ids with the alias
+            # --- Add new field to the shim for the alias to be associated to the
+            # --- existing ID. This augmentation of the dimension dict it a temporary
+            # --- hack for subvariable ref that should change in future
             for idx, alias in enumerate(self._subvar_aliases):
-                shim["type"]["elements"][idx]["id"] = alias
+                shim["type"]["elements"][idx]["subvar_alias"] = alias
 
         # --- datetimes are identified by their value
         if self.dimension_type == DT.DATETIME:
             for el in shim["type"]["elements"]:
                 # --- Missing data comes in as a dict and should be ignored
                 if not isinstance(el["value"], dict):
-                    el["id"] = el["value"]
+                    # --- Add new field to the shim for datetime value to be associated
+                    # --- to the existing ID. This augmentation of the dimension dict
+                    # --- it a temporary hack for subvariable ref that should change
+                    # --- in future
+                    el["datetime_value"] = el["value"]
 
         # --- Leave other types alone
         return shim
 
     @lazyproperty
     def shimmed_dimension_transforms_dict(self) -> Dict:
-        """Copy of dimension transforms dictionary with shimmed `element_id`s
+        """Augmented dimension transforms dictionary with shimmed `element_id`s
 
         We want to move to a world where elements on a subvariables dimension are
         identified by their alias, but right now the "element_id" from zz9 is
@@ -675,7 +691,7 @@ def shimmed_dimension_transforms_dict(self) -> Dict:
         - "alias": Subvariables also have an alias that identifies them. It is stored
           in `dimensions[i].type.elements[j].value.references.alias`.
         """
-        shim = copy.deepcopy(self._dimension_transforms_dict)
+        shim = self._dimension_transforms_dict
 
         # --- Leave non-subvariable dimension types alone, as they don't have
         # --- subvariable aliases to use, and category ids are already the main way
@@ -896,11 +912,14 @@ class Element:
     This object resolves the transform cascade for element-level transforms.
     """
 
-    def __init__(self, element_dict, index, element_transforms, label_formatter):
+    def __init__(
+        self, element_dict, index, element_transforms, label_formatter, dim_type
+    ):
         self._element_dict = element_dict
         self._index = index
         self._element_transforms = element_transforms
         self._label_formatter = label_formatter
+        self._dim_type = dim_type
 
     def __repr__(self) -> str:
         """str of this element, which makes it easier to work in cosole."""
@@ -920,9 +939,9 @@ def anchor(self) -> Optional[Union[str, dict]]:
         return self._element_dict.get("value", {}).get("references", {}).get("anchor")
 
     @lazyproperty
-    def element_id(self) -> int:
+    def element_id(self) -> Union[int, str]:
         """int identifier for this category or subvariable."""
-        return self._element_dict["id"]
+        return _build_element_id(self._element_dict, self._dim_type)
 
     @lazyproperty
     def derived(self) -> bool:

tests/integration/test_dimension.py

@@ -295,7 +295,7 @@ class DescribeIntegrated_Element:
 
     def it_knows_its_transformed_label(self, element_dict, element_transforms_):
         element_transforms_.name = "Xfinity Lounge"
-        element = Element(element_dict, None, element_transforms_, str)
+        element = Element(element_dict, None, element_transforms_, str, DT.CATEGORICAL)
 
         label = element.label
 
@@ -305,31 +305,31 @@ def but_it_uses_its_base_name_if_no_transform_is_present(
         self, element_dict, element_transforms_
     ):
         element_transforms_.name = None
-        element = Element(element_dict, None, element_transforms_, None)
+        element = Element(element_dict, None, element_transforms_, None, DT.CATEGORICAL)
 
         label = element.label
 
         assert label == "President Obama"
 
     def it_knows_when_it_is_explicitly_hidden(self, element_dict, element_transforms_):
         element_transforms_.hide = True
-        element = Element(element_dict, None, element_transforms_, None)
+        element = Element(element_dict, None, element_transforms_, None, DT.CATEGORICAL)
 
         is_hidden = element.is_hidden
 
         assert is_hidden is True
 
     def but_it_is_not_hidden_by_default(self, element_transforms_):
         element_transforms_.hide = None
-        element = Element(None, None, element_transforms_, None)
+        element = Element(None, None, element_transforms_, None, DT.CATEGORICAL)
 
         is_hidden = element.is_hidden
 
         assert is_hidden is False
 
     def it_knows_how_to_repr(self, element_dict, element_transforms_):
         element_transforms_.name = None
-        element = Element(element_dict, None, element_transforms_, None)
+        element = Element(element_dict, None, element_transforms_, None, DT.CATEGORICAL)
 
         str_representation = str(element)
 

tests/unit/test_dimension.py

@@ -804,7 +804,10 @@ def it_provides_the_shimmed_dimension_dict_for_subvars(self, _subvar_aliases_pro
 
         assert shim_.shimmed_dimension_dict == {
             "type": {
-                "elements": [{"id": "alias1", "element_info": 100}, {"id": "alias2"}],
+                "elements": [
+                    {"id": 1, "element_info": 100, "subvar_alias": "alias1"},
+                    {"id": 2, "subvar_alias": "alias2"},
+                ],
                 "other": "in type",
             },
             "another": "outside type",
@@ -827,9 +830,14 @@ def and_it_provides_the_shimmed_dimension_dict_for_datetime(self):
         assert shim_.shimmed_dimension_dict == {
             "type": {
                 "elements": [
-                    {"id": "val1", "value": "val1", "element_info": 100},
+                    {
+                        "id": 1,
+                        "value": "val1",
+                        "element_info": 100,
+                        "datetime_value": "val1",
+                    },
                     {"id": 2, "value": {"?": -1}},
-                    {"id": "another val", "value": "another val"},
+                    {"id": 3, "value": "another val", "datetime_value": "another val"},
                 ],
                 "other": "in type",
             },
@@ -1096,38 +1104,38 @@ class Describe_Element:
 
     def it_knows_its_anchor(self):
         element_dict = {"value": {"references": {"anchor": "top"}, "derived": True}}
-        element = Element(element_dict, None, None, None)
+        element = Element(element_dict, None, None, None, None)
 
         anchor = element.anchor
 
         assert anchor == "top"
 
     def but_anchor_is_none_if_not_derived(self):
         element_dict = {"value": {"derived": False}}
-        element = Element(element_dict, None, None, None)
+        element = Element(element_dict, None, None, None, None)
 
         anchor = element.anchor
 
         assert anchor is None
 
     def it_knows_its_element_id(self):
         element_dict = {"id": 42}
-        element = Element(element_dict, None, None, None)
+        element = Element(element_dict, None, None, None, None)
 
         element_id = element.element_id
 
         assert element_id == 42
 
     def it_knows_its_fill_RGB_color_str(self, element_transforms_):
         element_transforms_.fill = [255, 255, 248]
-        element = Element(None, None, element_transforms_, None)
+        element = Element(None, None, element_transforms_, None, None)
 
         rgb_color_fill = element.fill
 
         assert rgb_color_fill == [255, 255, 248]
 
     def it_knows_its_position_among_all_the_dimension_elements(self):
-        element = Element(None, 17, None, None)
+        element = Element(None, 17, None, None, None)
         index = element.index
         assert index == 17
 
@@ -1162,15 +1170,15 @@ def it_knows_its_label(
         fmt_calls,
     ):
         element_transforms_.name = transform_name
-        element = Element(element_dict, None, element_transforms_, str)
+        element = Element(element_dict, None, element_transforms_, str, None)
         assert element.label == expected_value
 
     @pytest.mark.parametrize(
         ("hide", "expected_value"), ((True, True), (False, False), (None, False))
     )
     def it_knows_whether_it_is_explicitly_hidden(self, request, hide, expected_value):
         element_transforms_ = instance_mock(request, _ElementTransforms, hide=hide)
-        element = Element(None, None, element_transforms_, None)
+        element = Element(None, None, element_transforms_, None, None)
 
         is_hidden = element.is_hidden
 
@@ -1189,7 +1197,7 @@ def it_knows_whether_it_is_explicitly_hidden(self, request, hide, expected_value
         ),
     )
     def it_knows_whether_its_missing_or_valid(self, element_dict, expected_value):
-        element = Element(element_dict, None, None, None)
+        element = Element(element_dict, None, None, None, None)
 
         missing = element.missing
 
@@ -1212,7 +1220,7 @@ def it_knows_whether_its_missing_or_valid(self, element_dict, expected_value):
         ),
     )
     def it_knows_its_numeric_value(self, element_dict, expected_value):
-        element = Element(element_dict, None, None, None)
+        element = Element(element_dict, None, None, None, None)
 
         numeric_value = element.numeric_value
 
@@ -1594,11 +1602,11 @@ def it_knows_the_addend_idxs(
         addend_ids_.return_value = addend_ids
         valid_elements = Elements(
             [
-                Element({"id": 1}, None, None, None),
-                Element({"id": 2}, None, None, None),
-                Element({"id": 3}, None, None, None),
-                Element({"id": 4}, None, None, None),
-                Element({"id": 99}, None, None, None),
+                Element({"id": 1}, None, None, None, None),
+                Element({"id": 2}, None, None, None, None),
+                Element({"id": 3}, None, None, None, None),
+                Element({"id": 4}, None, None, None, None),
+                Element({"id": 99}, None, None, None, None),
             ]
         )
         subtotal = _Subtotal(subtotal_dict, valid_elements)
@@ -1699,11 +1707,11 @@ def it_knows_the_subtrahend_idxs(
         subtrahend_ids_.return_value = subtrahend_ids
         valid_elements = Elements(
             [
-                Element({"id": 1}, None, None, None),
-                Element({"id": 2}, None, None, None),
-                Element({"id": 3}, None, None, None),
-                Element({"id": 4}, None, None, None),
-                Element({"id": 99}, None, None, None),
+                Element({"id": 1}, None, None, None, None),
+                Element({"id": 2}, None, None, None, None),
+                Element({"id": 3}, None, None, None, None),
+                Element({"id": 4}, None, None, None, None),
+                Element({"id": 99}, None, None, None, None),
             ]
         )
         subtotal = _Subtotal(subtotal_dict, valid_elements)