Set disabled TableLayerArtists to be not visible

[jfoster17]

Pull Request Template

Description

A fix for #2285, making sure that disabled TableLayerArtists are not visible, as the viewer will sometimes try to draw visible (but disabled) layers and throw an IncompatibleAttribute Error when it tries to do so. A regression test for this behavior is included.

[codecov]

Codecov Report

Merging #2286 (a86a64a) into main (e8f03f1) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2286      +/-   ##
==========================================
+ Coverage   88.13%   88.14%   +0.01%     
==========================================
  Files         247      247              
  Lines       23292    23293       +1     
==========================================
+ Hits        20528    20532       +4     
+ Misses       2764     2761       -3     

Impacted Files	Coverage Δ
glue/viewers/table/qt/data_viewer.py	97.42% <100.00%> (+0.01%)	⬆️
glue/conftest.py	67.50% <0.00%> (+3.75%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e8f03f1...a86a64a. Read the comment docs.

[jfoster17]

Thanks for your comments @dhomeier -- I don't think it matters if you call viewer.state.layers or viewer.layers since they should be kept in sync. I accidentally removed the calls to

gapp.show()
process_events()

when cleaning up this commit. Without this the layers don't try to be drawn and become invalid. With this restored you should be able to see this test failing without my change to data_by_row_and_column() and passing with my change.

[jfoster17]

But I also did change to call viewer.layers directly...

[dhomeier]

Thanks for your comments @dhomeier -- I don't think it matters if you call viewer.state.layers or viewer.layers since they should be kept in sync. I accidentally removed the calls to

I just noticed that viewer.state.layers unlike viewer.layers e.g. do not have a enabled attribute, so it was not clear to me how they would be kept in sync. But looks close to ready now, thanks!

[dhomeier]

I don't have an idea about the failure with pyqt59-legacy, but if you cannot find a solution, I'd find it acceptable to mark that as xfail.
The pyqt515-dev error is unrelated and fixed in #2287.
That leaves mainly the coding style to be addressed, and if you can think of a clearer description that would be a bonus (but I am not very familiar with that part of the code)!

[jfoster17]

I do not know the source of the pyqt59-legacy error. How do I mark that as xfail for a specific configuration?

[dhomeier]

I don't see any existing hooks for PYQT5 versions, so perhaps the best solution would be to define your own requires_pyqt_gt_59 with make_skipper in helpers.py analogously to

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

Line 641 in e8f03f1

@requires_matplotlib_ge_22

only this would have to additionally include not PYSIDE2_INSTALLED.
In theory the failure might be due to any of the legacy packages in that run, but since this is mostly working on the PyQt interface, that's probably the best bet – and would work either way for skipping the test in the CI.
Pinging @astrofrog about the best approach for this.

[jfoster17]

Ok -- so I've added a helper to skip this test for PyQt < 5.10, which means that pyqt59-legacy runs successfully now and the only failure is due to #2287. Note that I did have to make a more substantial edit to make_skipper because PyQt5 does not use __version__ for a version string. Happy to wait and see if @astrofrog would prefer another approach.

[dhomeier]

The make_skipper extension should be quite useful in general, thanks.
For my part this will be good to go once the test is no longer skipped for *-pyside*.

[astrofrog]

The test failures are unrelated, so I'll approve and merge now - thanks @jfoster17!

[dhomeier]

Thanks; yes, I haven't seen those pyside14 segfaults before; but everything is passing or skipping the new test now as it should.