From 06f936ff4fef745768712d17831a88c82f11b8e9 Mon Sep 17 00:00:00 2001 From: Ernesto Arbitrio Date: Tue, 10 Dec 2024 16:41:19 -0500 Subject: [PATCH 1/4] implement sory by label on col dimension --- src/cr/cube/matrix/assembler.py | 73 ++++++++++++++++++++++++++++++ tests/integration/test_cubepart.py | 42 +++++++++++++++++ 2 files changed, 115 insertions(+) diff --git a/src/cr/cube/matrix/assembler.py b/src/cr/cube/matrix/assembler.py index 7f3cf1abe..059777070 100644 --- a/src/cr/cube/matrix/assembler.py +++ b/src/cr/cube/matrix/assembler.py @@ -49,6 +49,12 @@ def column_display_order(cls, dimensions, second_order_measures): # --- _ColumnOrderHelper, so there's not much to this yet, just keeping # --- form consistent with `.row_display_order()` and we'll elaborate this when # --- we add sort-by-value to columns. + collation_method = dimensions[1].order_spec.collation_method + if collation_method == CM.LABEL: + return _SortColumnsByLabelHelper( + dimensions, second_order_measures, format + )._display_order + return _ColumnOrderHelper(dimensions, second_order_measures)._display_order @classmethod @@ -349,6 +355,73 @@ def _subtotal_values(self): ) +class _BaseSortColumnsByValueHelper(_ColumnOrderHelper): + """A base class that orders elements by a set of values. + + This class is intended only to serve as a base for the other sort-by-value classes + which must all provide their own implentations of `_element_values` and + `_subtotal_values`. + + If `_order` encouters a ValueError, it falls back to the payload order. + """ + + @lazyproperty + def _element_values(self): + """Sequence of body values that form the basis for sort order. + + Must be implemented by child classes. + """ + raise NotImplementedError( # pragma: no cover + f"{type(self).__name__} must implement `._element_values`" + ) + + @lazyproperty + def _order(self): + """tuple of int element-idx specifying ordering of dimension elements.""" + try: + return SortByValueCollator.display_order( + self._columns_dimension, + self._element_values, + self._subtotal_values, + self._empty_column_idxs, + self._format, + ) + except ValueError: + return PayloadOrderCollator.display_order( + self._columns_dimension, self._empty_column_idxs, self._format + ) + + @lazyproperty + def _subtotal_values(self): + """Sequence of subtotal values that form the basis for sort order. + + Must be implemented by child classes. + """ + raise NotImplementedError( # pragma: no cover + f"{type(self).__name__} must implement `._subtotal_values`" + ) + + +class _SortColumnsByLabelHelper(_BaseSortColumnsByValueHelper): + @lazyproperty + def _element_values(self): + """Sequence of body labels that form the basis for sort order. + + There is one value per column and values appear in payload (dimension) element + order. These are only the "base" values and do not include insertions. + """ + return np.array(self._dimensions[1].element_labels) + + @lazyproperty + def _subtotal_values(self): + """Sequence of col-subtotal labels that contribute to the sort basis. + + There is one value per column subtotal and labels appear in payload (dimension) + insertion order. + """ + return np.array(self._dimensions[1].subtotal_labels) + + class _SortRowsByBaseColumnHelper(_BaseSortRowsByValueHelper): """Orders elements by the values of an opposing base (not a subtotal) vector. diff --git a/tests/integration/test_cubepart.py b/tests/integration/test_cubepart.py index 939aa4d18..9108b9f57 100644 --- a/tests/integration/test_cubepart.py +++ b/tests/integration/test_cubepart.py @@ -2,6 +2,7 @@ """Integration-test suite for `cr.cube.cubepart` module.""" +from textwrap import dedent import numpy as np import pytest @@ -921,6 +922,47 @@ def test_and_it_respects_explicit_order_transform_for_measures( expected = load_python_expression(expectation) np.testing.assert_almost_equal(actual, expected) + def test_it_can_sort_by_column_by_labels(self): + transforms = { + "columns_dimension": {"order": {"type": "label", "direction": "ascending"}} + } + slice_ = _Slice(Cube(CR.CAT_4_X_CAT_5), 0, transforms, None, 0) + expected_slice_repr = """ + _Slice(name='Support', dimension_types='CAT x CAT') + Showing: COUNT + As married Divorced Married Never Widowed + ---------- ------------ ---------- --------- ------- --------- + Plenty 21 3 46 7 0 + Enough 55 13 127 29 1 + Not enough 17 41 253 47 1 + N/A 80 19 247 26 4 + Available measures: [] + """ + assert slice_.__repr__() == dedent(expected_slice_repr).strip() + + transforms = { + "columns_dimension": { + "order": { + "type": "label", + "direction": "ascending", + "fixed": {"bottom": [1]}, + } + } + } + slice_ = _Slice(Cube(CR.CAT_4_X_CAT_5), 0, transforms, None, 0) + expected_slice_repr = """ + _Slice(name='Support', dimension_types='CAT x CAT') + Showing: COUNT + As married Divorced Never Widowed Married + ---------- ------------ ---------- ------- --------- --------- + Plenty 21 3 7 0 46 + Enough 55 13 29 1 127 + Not enough 17 41 47 1 253 + N/A 80 19 26 4 247 + Available measures: [] + """ + assert slice_.__repr__() == dedent(expected_slice_repr).strip() + def test_it_can_sort_by_column_index(self): """Responds to order:opposing_element sort-by-column-index.""" transforms = { From e6590abdc36867f1ad9582a81b2d13acaa2d3315 Mon Sep 17 00:00:00 2001 From: Ernesto Arbitrio Date: Tue, 10 Dec 2024 16:46:46 -0500 Subject: [PATCH 2/4] unit tests --- tests/unit/matrix/test_assembler.py | 112 ++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/tests/unit/matrix/test_assembler.py b/tests/unit/matrix/test_assembler.py index 592ae6a56..729827755 100644 --- a/tests/unit/matrix/test_assembler.py +++ b/tests/unit/matrix/test_assembler.py @@ -18,9 +18,11 @@ ) from cr.cube.matrix.assembler import ( _BaseOrderHelper, + _BaseSortColumnsByValueHelper, _BaseSortRowsByValueHelper, _ColumnOrderHelper, _RowOrderHelper, + _SortColumnsByLabelHelper, _SortRowsByBaseColumnHelper, _SortRowsByDerivedColumnHelper, _SortRowsByInsertedColumnHelper, @@ -1393,6 +1395,84 @@ def _subtotal_values_prop_(self, request): return property_mock(request, _BaseSortRowsByValueHelper, "_subtotal_values") +class Test_BaseSortColumnsByValueHelper: + """Unit test suite for `cr.cube.matrix.assembler._BaseSortColumnsByValueHelper`.""" + + def test_it_provides_the_order( + self, + SortByValueCollator_, + _columns_dimension_prop_, + _element_values_prop_, + _subtotal_values_prop_, + _empty_column_idxs_prop_, + ): + _BaseSortColumnsByValueHelper(None, None)._order + + SortByValueCollator_.display_order.assert_called_once_with( + _columns_dimension_prop_(), + _element_values_prop_(), + _subtotal_values_prop_(), + _empty_column_idxs_prop_(), + ORDER_FORMAT.SIGNED_INDEXES, + ) + + def test_but_it_falls_back_to_payload_order_on_value_error( + self, + request, + dimensions_, + _element_values_prop_, + _subtotal_values_prop_, + _empty_column_idxs_prop_, + SortByValueCollator_, + ): + _element_values_prop_.return_value = None + _subtotal_values_prop_.return_value = None + _empty_column_idxs_prop_.return_value = (4, 2) + SortByValueCollator_.display_order.side_effect = ValueError + PayloadOrderCollator_ = class_mock( + request, "cr.cube.matrix.assembler.PayloadOrderCollator" + ) + PayloadOrderCollator_.display_order.return_value = (1, 2, 3, 4) + order_helper = _BaseSortColumnsByValueHelper(dimensions_, None) + + order = order_helper._order + + PayloadOrderCollator_.display_order.assert_called_once_with( + dimensions_[1], (4, 2), ORDER_FORMAT.SIGNED_INDEXES + ) + assert order == (1, 2, 3, 4) + + # fixture components --------------------------------------------- + + @pytest.fixture + def dimensions_(self, request): + return (instance_mock(request, Dimension), instance_mock(request, Dimension)) + + @pytest.fixture + def _element_values_prop_(self, request): + return property_mock(request, _BaseSortColumnsByValueHelper, "_element_values") + + @pytest.fixture + def _empty_column_idxs_prop_(self, request): + return property_mock( + request, _BaseSortColumnsByValueHelper, "_empty_column_idxs" + ) + + @pytest.fixture + def _columns_dimension_prop_(self, request): + return property_mock( + request, _BaseSortColumnsByValueHelper, "_columns_dimension" + ) + + @pytest.fixture + def SortByValueCollator_(self, request): + return class_mock(request, "cr.cube.matrix.assembler.SortByValueCollator") + + @pytest.fixture + def _subtotal_values_prop_(self, request): + return property_mock(request, _BaseSortColumnsByValueHelper, "_subtotal_values") + + class Test_SortRowsByBaseColumnHelper: """Unit test suite for `cr.cube.matrix.assembler._SortRowsByBaseColumnHelper`.""" @@ -1606,6 +1686,38 @@ def dimensions_(self, request): return (instance_mock(request, Dimension), instance_mock(request, Dimension)) +class Test_SortColumnsByLabelHelper: + """Unit test suite for `cr.cube.matrix.assembler._SortColumnsByLabelHelper`.""" + + def test_it_provides_the_element_values_to_help(self, dimensions_): + dimensions_[1].element_labels = ("c", "a", "b") + + assert _SortColumnsByLabelHelper( + dimensions_, None + )._element_values.tolist() == [ + "c", + "a", + "b", + ] + + def test_it_provides_the_subtotal_values_to_help(self, dimensions_): + dimensions_[1].subtotal_labels = ("c", "a", "b") + + assert _SortColumnsByLabelHelper( + dimensions_, None + )._subtotal_values.tolist() == [ + "c", + "a", + "b", + ] + + # fixture components --------------------------------------------- + + @pytest.fixture + def dimensions_(self, request): + return (instance_mock(request, Dimension), instance_mock(request, Dimension)) + + class Test_SortRowsByMarginalHelper: """Unit test suite for `cr.cube.matrix.assembler._SortRowsByMarginalHelper`.""" From ac794a08ca78ef98f17a5a6a9ae7e7a1068c3fbc Mon Sep 17 00:00:00 2001 From: Ernesto Arbitrio Date: Wed, 11 Dec 2024 09:30:16 -0500 Subject: [PATCH 3/4] integration test for fixed order --- tests/integration/test_cubepart.py | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/tests/integration/test_cubepart.py b/tests/integration/test_cubepart.py index 9108b9f57..b7d278852 100644 --- a/tests/integration/test_cubepart.py +++ b/tests/integration/test_cubepart.py @@ -1058,9 +1058,28 @@ def test_it_can_sort_by_marginal_with_nan_in_body(self): [2.45356177, 2.11838791, 2.0, 1.97, 1.74213625, np.nan], nan_ok=True ) - def test_it_can_fix_order_of_subvars_identified_by_bogus_id(self): + @pytest.mark.parametrize( + "dimension, expceted_labels", + ( + ("rows_dimension", ["Finland", "Sweden", "Norway", "Denmark", "Iceland"]), + ( + "columns_dimension", + [ + "Chitarra", + "Quadrefiore", + "Orecchiette", + "Fileja", + "Bucatini", + "Boccoli", + ], + ), + ), + ) + def test_it_can_fix_order_of_subvars_identified_by_bogus_id( + self, dimension, expceted_labels + ): transforms = { - "rows_dimension": { + dimension: { "order": { "type": "label", "direction": "descending", @@ -1070,10 +1089,9 @@ def test_it_can_fix_order_of_subvars_identified_by_bogus_id(self): } } slice_ = _Slice(Cube(CR.MR_X_CAT), 0, transforms, None, 0) - - expected = ["Finland", "Sweden", "Norway", "Denmark", "Iceland"] - actual = slice_.row_labels.tolist() - assert expected == actual, "\n%s\n\n%s" % (expected, actual) + prop = "row_labels" if dimension == "rows_dimension" else "column_labels" + actual = getattr(slice_, prop).tolist() + assert expceted_labels == actual, "\n%s\n\n%s" % (expceted_labels, actual) @pytest.mark.parametrize( "id", From d5bb005c17011ee865670c877b2cedf5b29d1c48 Mon Sep 17 00:00:00 2001 From: Ernesto Arbitrio Date: Thu, 12 Dec 2024 12:45:59 -0500 Subject: [PATCH 4/4] address cr comments --- tests/integration/test_cubepart.py | 49 +++++++++++++++--------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/tests/integration/test_cubepart.py b/tests/integration/test_cubepart.py index b7d278852..b50c4e56d 100644 --- a/tests/integration/test_cubepart.py +++ b/tests/integration/test_cubepart.py @@ -1023,7 +1023,7 @@ def test_it_can_sort_by_column_percent(self): actual = np.round(slice_.column_percentages, 1).tolist() assert expected == actual, "\n%s\n\n%s" % (expected, actual) - def test_it_can_sort_by_labels(self): + def test_it_can_sort_rows_by_labels(self): """Responds to order:label sort-by-label.""" transforms = { "rows_dimension": { @@ -1058,28 +1058,9 @@ def test_it_can_sort_by_marginal_with_nan_in_body(self): [2.45356177, 2.11838791, 2.0, 1.97, 1.74213625, np.nan], nan_ok=True ) - @pytest.mark.parametrize( - "dimension, expceted_labels", - ( - ("rows_dimension", ["Finland", "Sweden", "Norway", "Denmark", "Iceland"]), - ( - "columns_dimension", - [ - "Chitarra", - "Quadrefiore", - "Orecchiette", - "Fileja", - "Bucatini", - "Boccoli", - ], - ), - ), - ) - def test_it_can_fix_order_of_subvars_identified_by_bogus_id( - self, dimension, expceted_labels - ): + def test_it_can_fix_row_order_of_subvars_identified_by_bogus_id(self): transforms = { - dimension: { + "rows_dimension": { "order": { "type": "label", "direction": "descending", @@ -1089,9 +1070,27 @@ def test_it_can_fix_order_of_subvars_identified_by_bogus_id( } } slice_ = _Slice(Cube(CR.MR_X_CAT), 0, transforms, None, 0) - prop = "row_labels" if dimension == "rows_dimension" else "column_labels" - actual = getattr(slice_, prop).tolist() - assert expceted_labels == actual, "\n%s\n\n%s" % (expceted_labels, actual) + + expected = ["Finland", "Sweden", "Norway", "Denmark", "Iceland"] + actual = slice_.row_labels.tolist() + assert expected == actual, "\n%s\n\n%s" % (expected, actual) + + def test_it_can_fix_column_order_of_subvars_identified_by_bogus_id(self): + transforms = { + "columns_dimension": { + "order": { + "type": "label", + "direction": "descending", + # --- Front-end uses idx to refer to MR subvariables + "fixed": {"bottom": [0]}, + } + } + } + slice_ = _Slice(Cube(MRI.CAT_X_MR), 0, transforms, None, 0) + + expected = ["Response #3", "Response #2", "Response #1", "A&B"] + actual = slice_.column_labels.tolist() + assert expected == actual, "\n%s\n\n%s" % (expected, actual) @pytest.mark.parametrize( "id",