Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove deepcopy from the codebase #395

Merged
merged 3 commits into from
Feb 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 33 additions & 14 deletions src/cr/cube/dimension.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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."""

Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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
ernestoarbitrio marked this conversation as resolved.
Show resolved Hide resolved

# --- 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
Expand All @@ -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
ernestoarbitrio marked this conversation as resolved.
Show resolved Hide resolved

# --- Leave non-subvariable dimension types alone, as they don't have
# --- subvariable aliases to use, and category ids are already the main way
Expand Down Expand Up @@ -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
ernestoarbitrio marked this conversation as resolved.
Show resolved Hide resolved
):
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."""
Expand All @@ -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:
Expand Down
10 changes: 5 additions & 5 deletions tests/integration/test_dimension.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)

Expand Down
52 changes: 30 additions & 22 deletions tests/unit/test_dimension.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
},
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
Loading