From 9203ef87e629c8b8ba9ffc4ad7394a5f5325a4ff Mon Sep 17 00:00:00 2001 From: Jonathan Foster Date: Fri, 1 Dec 2023 15:32:23 -0500 Subject: [PATCH 1/2] Catch IncompatibleAttribute for subset-only tables and show newly-valid subsets --- glue_qt/viewers/table/data_viewer.py | 24 ++++++++++- .../viewers/table/tests/test_data_viewer.py | 41 ++++++++++++++++--- 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/glue_qt/viewers/table/data_viewer.py b/glue_qt/viewers/table/data_viewer.py index d76d8e14..66b6ad81 100644 --- a/glue_qt/viewers/table/data_viewer.py +++ b/glue_qt/viewers/table/data_viewer.py @@ -174,9 +174,24 @@ def _update_visible(self): Given which layers are visible or not, convert order to order_visible after applying the current filter_mask """ - self.data_by_row_and_column.cache_clear() + # We want to make visible subsets that were previously invisible + # because they were incompatible subsets. This requires some + # extra calls to to_mask() and could probably be optimized. + + for layer_artist in self._table_viewer.layers: + if isinstance(layer_artist.layer, BaseData): + continue + + if not layer_artist.enabled: + try: + mask = layer_artist.layer.to_mask() + except IncompatibleAttribute: + continue + layer_artist.enabled = True + layer_artist.visible = True + # First, if the data layer is visible, show all rows for layer_artist in self._table_viewer.layers: if layer_artist.visible and isinstance(layer_artist.layer, BaseData): @@ -192,7 +207,11 @@ def _update_visible(self): visible = np.zeros(self.order.shape, dtype=bool) for layer_artist in self._table_viewer.layers: if layer_artist.visible: - mask = layer_artist.layer.to_mask()[self.order] + try: + mask = layer_artist.layer.to_mask()[self.order] + except IncompatibleAttribute: + layer_artist.disable_incompatible_subset() + continue if DASK_INSTALLED and isinstance(mask, da.Array): mask = mask.compute() visible |= mask @@ -221,6 +240,7 @@ def update(self): self._refresh() def clear(self): + self.visible = False self._refresh() diff --git a/glue_qt/viewers/table/tests/test_data_viewer.py b/glue_qt/viewers/table/tests/test_data_viewer.py index 84371408..598c92a1 100644 --- a/glue_qt/viewers/table/tests/test_data_viewer.py +++ b/glue_qt/viewers/table/tests/test_data_viewer.py @@ -13,7 +13,6 @@ from ..data_viewer import DataTableModel, TableViewer from glue.core.edit_subset_mode import AndNotMode, OrMode, ReplaceMode -from glue.tests.helpers import requires_pyqt_gt_59_or_pyside2 class TestDataTableModel(): @@ -461,7 +460,6 @@ def test_incompatible_subset(): assert refresh2.call_count == 0 -@requires_pyqt_gt_59_or_pyside2 def test_table_incompatible_attribute(): """ Regression test for a bug where the table viewer generates an @@ -487,24 +485,57 @@ def test_table_incompatible_attribute(): process_events() assert len(viewer.layers) == 2 - assert not viewer.layers[1].visible assert viewer.layers[0].visible + assert not viewer.layers[1].visible # This subset can be shown in the viewer sg2 = dc.new_subset_group('valid', d2.id['a'] == 'a') + process_events() + assert len(viewer.layers) == 3 - assert viewer.layers[2].visible - assert not viewer.layers[1].visible assert viewer.layers[0].visible + assert not viewer.layers[1].visible + assert viewer.layers[2].visible # The original IncompatibleAttribute was thrown # here as making the data layer invisible made # DataTableModel._update_visible() try and draw # the invalid subset viewer.layers[0].visible = False + + process_events() + + assert not viewer.layers[0].visible + assert not viewer.layers[1].visible assert viewer.layers[2].visible + + # Another IncompatibleAttribute was thrown here + # as an invalid subset was attempted to be added + # to a table viewer showing only a subset + + sg3 = dc.new_subset_group('invalid', d1.id['y'] > 6) + + process_events() + + assert len(viewer.layers) == 4 + assert not viewer.layers[0].visible assert not viewer.layers[1].visible + assert viewer.layers[2].visible + assert not viewer.layers[3].visible + + # A previously disabled layer artist should + # be renabled when it can be visualized + + sg1.subset_state = d2.id['b'] == 'b' + + process_events() + + assert len(viewer.layers) == 4 + assert not viewer.layers[0].visible + assert viewer.layers[1].visible + assert viewer.layers[2].visible + assert not viewer.layers[3].visible def test_table_with_dask_column(): From 11eeadddbbaffde4751561e2e1a7f83f2443c287 Mon Sep 17 00:00:00 2001 From: Jonathan Foster Date: Fri, 1 Dec 2023 16:46:55 -0500 Subject: [PATCH 2/2] Increase process_events time to try and solve CI failures --- glue_qt/viewers/table/tests/test_data_viewer.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/glue_qt/viewers/table/tests/test_data_viewer.py b/glue_qt/viewers/table/tests/test_data_viewer.py index 598c92a1..e3eb883b 100644 --- a/glue_qt/viewers/table/tests/test_data_viewer.py +++ b/glue_qt/viewers/table/tests/test_data_viewer.py @@ -482,7 +482,7 @@ def test_table_incompatible_attribute(): sg1 = dc.new_subset_group('invalid', d1.id['x'] <= 3) gapp.show() - process_events() + process_events(0.5) assert len(viewer.layers) == 2 assert viewer.layers[0].visible @@ -491,7 +491,7 @@ def test_table_incompatible_attribute(): # This subset can be shown in the viewer sg2 = dc.new_subset_group('valid', d2.id['a'] == 'a') - process_events() + process_events(0.5) assert len(viewer.layers) == 3 assert viewer.layers[0].visible @@ -504,7 +504,7 @@ def test_table_incompatible_attribute(): # the invalid subset viewer.layers[0].visible = False - process_events() + process_events(0.5) assert not viewer.layers[0].visible assert not viewer.layers[1].visible @@ -516,7 +516,7 @@ def test_table_incompatible_attribute(): sg3 = dc.new_subset_group('invalid', d1.id['y'] > 6) - process_events() + process_events(0.5) assert len(viewer.layers) == 4 assert not viewer.layers[0].visible @@ -529,7 +529,7 @@ def test_table_incompatible_attribute(): sg1.subset_state = d2.id['b'] == 'b' - process_events() + process_events(0.5) assert len(viewer.layers) == 4 assert not viewer.layers[0].visible