From d7eaab0de3c3ff17893bad3771203a89dbfd8cdf Mon Sep 17 00:00:00 2001 From: Slobodan Ilic Date: Tue, 19 Mar 2019 09:12:32 +0100 Subject: [PATCH 1/2] Eliminate `memoize` from Dimension.py * Make `all_elements` and `valid_elements` parts of the interface * Use newly available properties from `CrunchCube` * Fix unit tests --- src/cr/cube/crunch_cube.py | 11 +++++-- src/cr/cube/dimension.py | 60 +++++++++++++----------------------- tests/unit/test_dimension.py | 2 +- 3 files changed, 30 insertions(+), 43 deletions(-) diff --git a/src/cr/cube/crunch_cube.py b/src/cr/cube/crunch_cube.py index 5880206b1..ce776a5c8 100644 --- a/src/cr/cube/crunch_cube.py +++ b/src/cr/cube/crunch_cube.py @@ -359,7 +359,7 @@ def hs_dims_for_den(hs_dims, axis): if ( d.dimension_type == DT.MR_CAT or i != 0 - and (n <= 1 or len(d.elements()) <= 1) + and (n <= 1 or len(d.valid_elements) <= 1) ) else slice(None) ) @@ -831,7 +831,12 @@ def _apply_missings_and_insertions( # --element idxs that satisfy `include_missing` arg. Note this # --includes MR_CAT elements so is essentially all-or-valid-elements element_idxs = tuple( - d.element_indices(include_missing) for d in self._all_dimensions + ( + d.all_elements.element_idxs + if include_missing + else d.valid_elements.element_idxs + ) + for d in self._all_dimensions ) if not include_transforms_for_dims: return res[np.ix_(*element_idxs)] if element_idxs else res @@ -990,7 +995,7 @@ def _drop_mr_cat_dims(self, array, fix_valids=False): if not fix_valids else np.ix_( *[ - dim.element_indices(include_missing=False) if n > 1 else [0] + dim.valid_elements.element_idxs if n > 1 else [0] for dim, n in zip(self._all_dimensions, array.shape) ] ) diff --git a/src/cr/cube/dimension.py b/src/cr/cube/dimension.py index 24574fe53..1a9d40ea1 100644 --- a/src/cr/cube/dimension.py +++ b/src/cr/cube/dimension.py @@ -7,7 +7,7 @@ import numpy as np from cr.cube.enum import DIMENSION_TYPE as DT -from cr.cube.util import lazyproperty, memoize +from cr.cube.util import lazyproperty class _BaseDimensions(Sequence): @@ -266,24 +266,6 @@ def dimension_type(self): """Member of DIMENSION_TYPE appropriate to this cube dimension.""" return self._dimension_type - @memoize - def element_indices(self, include_missing): - """Return tuple of int element idxs for this dimension. - - *include_missing* determines whether missing elements are included or - only valid element index values are returned. - """ - return ( - self._all_elements.element_idxs - if include_missing - else self._valid_elements.element_idxs - ) - - @memoize - def elements(self, include_missing=False): - """_Elements object providing access to elements of this dimension.""" - return self._all_elements if include_missing else self._valid_elements - @lazyproperty def has_transforms(self): """True if there are subtotals on this dimension, False otherwise.""" @@ -327,7 +309,7 @@ def inserted_hs_indices(self): return [ idx for idx, item in enumerate( - self._iter_interleaved_items(self._valid_elements) + self._iter_interleaved_items(self.valid_elements) ) if item.is_insertion ] @@ -351,7 +333,7 @@ def labels( # that effectively squashes what should be two methods into one. # Either get rid of the need for that alternate return value type or # create a separate method for it. - elements = self._all_elements if include_missing else self._valid_elements + elements = self.all_elements if include_missing else self.valid_elements include_subtotals = include_transforms and self.dimension_type != DT.CA_SUBVAR @@ -395,14 +377,14 @@ def numeric_values(self): a value, but an element with no numeric value appears as `np.nan` in the returned list. """ - return tuple(element.numeric_value for element in self._valid_elements) + return tuple(element.numeric_value for element in self.valid_elements) @lazyproperty def shape(self): - return len(self._all_elements) + return len(self.all_elements) @lazyproperty - def _all_elements(self): + def all_elements(self): """_AllElements object providing cats or subvars of this dimension.""" return _AllElements(self._dimension_dict["type"]) @@ -416,7 +398,7 @@ def _iter_interleaved_items(self, elements): Only elements in the passed *elements* collection appear, which allows control over whether missing elements are included by choosing - `._all_elements` or `._valid_elements`. + `.all_elements` or `.valid_elements`. """ subtotals = self._subtotals @@ -443,17 +425,17 @@ def _subtotals(self): insertion_dicts = ( [] if view is None else view.get("transform", {}).get("insertions", []) ) - return _Subtotals(insertion_dicts, self._valid_elements) + return _Subtotals(insertion_dicts, self.valid_elements) @lazyproperty - def _valid_elements(self): + def valid_elements(self): """_Elements object providing access to non-missing elements. Any categories or subvariables representing missing data are excluded from the collection; this sequence represents a subset of that - provided by `._all_elements`. + provided by `.all_elements`. """ - return self._all_elements.valid_elements + return self.all_elements.valid_elements class _BaseElements(Sequence): @@ -558,12 +540,12 @@ class _ValidElements(_BaseElements): """ def __init__(self, all_elements): - self._all_elements = all_elements + self.all_elements = all_elements @lazyproperty def _elements(self): """tuple containing actual sequence of element objects.""" - return tuple(element for element in self._all_elements if not element.missing) + return tuple(element for element in self.all_elements if not element.missing) class _BaseElement(object): @@ -667,7 +649,7 @@ class _Subtotals(Sequence): def __init__(self, insertion_dicts, valid_elements): self._insertion_dicts = insertion_dicts - self._valid_elements = valid_elements + self.valid_elements = valid_elements def __getitem__(self, idx_or_slice): """Implements indexed access.""" @@ -688,7 +670,7 @@ def iter_for_anchor(self, anchor): @lazyproperty def _element_ids(self): """frozenset of int id of each non-missing cat or subvar in dim.""" - return frozenset(self._valid_elements.element_ids) + return frozenset(self.valid_elements.element_ids) def _iter_valid_subtotal_dicts(self): """Generate each insertion dict that represents a valid subtotal.""" @@ -717,7 +699,7 @@ def _iter_valid_subtotal_dicts(self): def _subtotals(self): """Composed tuple storing actual sequence of _Subtotal objects.""" return tuple( - _Subtotal(subtotal_dict, self._valid_elements) + _Subtotal(subtotal_dict, self.valid_elements) for subtotal_dict in self._iter_valid_subtotal_dicts() ) @@ -727,7 +709,7 @@ class _Subtotal(object): def __init__(self, subtotal_dict, valid_elements): self._subtotal_dict = subtotal_dict - self._valid_elements = valid_elements + self.valid_elements = valid_elements @lazyproperty def anchor(self): @@ -744,7 +726,7 @@ def anchor(self): anchor = self._subtotal_dict["anchor"] try: anchor = int(anchor) - if anchor not in self._valid_elements.element_ids: + if anchor not in self.valid_elements.element_ids: return "bottom" return anchor except (TypeError, ValueError): @@ -759,7 +741,7 @@ def anchor_idx(self): anchor = self.anchor if anchor in ["top", "bottom"]: return anchor - return self._valid_elements.get_by_id(anchor).index + return self.valid_elements.get_by_id(anchor).index @lazyproperty def addend_ids(self): @@ -771,7 +753,7 @@ def addend_ids(self): return tuple( arg for arg in self._subtotal_dict.get("args", []) - if arg in self._valid_elements.element_ids + if arg in self.valid_elements.element_ids ) @lazyproperty @@ -783,7 +765,7 @@ def addend_idxs(self): rather than its element id. """ return tuple( - self._valid_elements.get_by_id(addend_id).index + self.valid_elements.get_by_id(addend_id).index for addend_id in self.addend_ids ) diff --git a/tests/unit/test_dimension.py b/tests/unit/test_dimension.py index f4c186981..a17b9dee6 100644 --- a/tests/unit/test_dimension.py +++ b/tests/unit/test_dimension.py @@ -619,7 +619,7 @@ def valid_elements_(self, request): @pytest.fixture def _valid_elements_prop_(self, request): - return property_mock(request, Dimension, "_valid_elements") + return property_mock(request, Dimension, "valid_elements") class Describe_BaseElements(object): From 6318918f98d1288baeccc6c1b573657fe9d68d58 Mon Sep 17 00:00:00 2001 From: Slobodan Ilic Date: Tue, 19 Mar 2019 14:43:21 +0100 Subject: [PATCH 2/2] Don't cover `lru_cache` in pytest --- src/cr/cube/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cr/cube/util.py b/src/cr/cube/util.py index 92ae68fe0..740b3b9d6 100644 --- a/src/cr/cube/util.py +++ b/src/cr/cube/util.py @@ -156,7 +156,7 @@ def __set__(self, obj, value): raise AttributeError("can't set attribute") -def lru_cache(maxsize=100): +def lru_cache(maxsize=100): # pragma: no cover """Least-recently-used cache decorator. Arguments to the cached function must be hashable.