Merge pull request #2286 from jfoster17/fix-table-viewer-with-incompa…

…tible-subsets

Set disabled TableLayerArtists to be not visible

glue/tests/helpers.py

@@ -12,9 +12,14 @@
 def make_skipper(module, label=None, version=None):
     label = label or module
     try:
-        mod = __import__(module)
+        if label == 'PyQt5':  # PyQt5 does not use __version__
+            from PyQt5 import QtCore
+            version_installed = QtCore.PYQT_VERSION_STR
+        else:
+            mod = __import__(module)
+            version_installed = mod.__version__
         if version:
-            assert LooseVersion(mod.__version__) >= LooseVersion(version)
+            assert LooseVersion(version_installed) >= LooseVersion(version)
         installed = True
     except (ImportError, AssertionError):
         installed = False
@@ -66,6 +71,11 @@ def make_skipper(module, label=None, version=None):
 requires_qt = pytest.mark.skipif(str(not QT_INSTALLED),
                                  reason='An installation of Qt is required')
 
+PYQT_GT_59, _ = make_skipper('PyQt5', version='5.10')
+
+requires_pyqt_gt_59_or_pyside2 = pytest.mark.skipif(str(not PYQT_GT_59 and not PYSIDE2_INSTALLED),
+                                                    reason='Requires PyQt > 5.9 or PySide2')
+
 
 @contextmanager
 def make_file(contents, suffix, decompress=False):

glue/viewers/table/qt/data_viewer.py

@@ -126,8 +126,10 @@ def data_by_row_and_column(self, row, column, role):
                         # Only disable the layer if enabled, as otherwise we
                         # will recursively call clear and _refresh, causing
                         # an infinite loop and performance issues.
+                        # Also make sure that a disabled layer is not visible
                         if layer_artist.enabled:
                             layer_artist.disable_invalid_attributes(*exc.args)
+                            layer_artist.visible = False
                     else:
                         layer_artist.enabled = True
 

glue/viewers/table/qt/tests/test_data_viewer.py

@@ -13,6 +13,7 @@
 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():
@@ -460,6 +461,52 @@ 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
+    uncaught IncompatibleAttribute error in _update_visible() if
+    the dataset is not visible and an invalid subset exists at all.
+    This occurred because layer_artists depending on
+    invalid attributes were only being disabled (but were still
+    visible) and the viewer attempts to compute a mask for
+    all visible subsets if the underlying dataset is not visible.
+    """
+    app = get_qapp()
+    d1 = Data(x=[1, 2, 3, 4], y=[5, 6, 7, 8], label='d1')
+    d2 = Data(a=['a', 'b', 'c'], b=['x', 'y', 'z'], label='d2')
+    dc = DataCollection([d1, d2])
+    gapp = GlueApplication(dc)
+    viewer = gapp.new_data_viewer(TableViewer)
+    viewer.add_data(d2)
+
+    # This subset should not be shown in the viewer
+    sg1 = dc.new_subset_group('invalid', d1.id['x'] <= 3)
+
+    gapp.show()
+    process_events()
+
+    assert len(viewer.layers) == 2
+    assert not viewer.layers[1].visible
+    assert viewer.layers[0].visible
+
+    # This subset can be shown in the viewer
+    sg2 = dc.new_subset_group('valid', d2.id['a'] == 'a')
+
+    assert len(viewer.layers) == 3
+    assert viewer.layers[2].visible
+    assert not viewer.layers[1].visible
+    assert viewer.layers[0].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
+    assert viewer.layers[2].visible
+    assert not viewer.layers[1].visible
+
+
 def test_table_with_dask_column():
 
     da = pytest.importorskip('dask.array')