glue-viz / glue-vispy-viewers

3-d data viewers for glue based on VisPy
http://glueviz.org/en/stable/whatsnew/experimental_3d.html#experimental-3d
BSD 2-Clause "Simplified" License
25 stars 21 forks source link

Update API to support `echo>=0.6` and `vispy=0.11` #373

Closed dhomeier closed 2 years ago

dhomeier commented 2 years ago

Description

This adds fixes for some upstream API changes which should fix a large part of the currently failing tests:

  1. list has been removed from echo
  2. https://github.com/vispy/vispy/pull/2227#issuecomment-953934640 ; pulling in 9034cd3 from #371 for this.
  3. vispy 0.10 -> 0.11 has replaced the CatRom filter with CatRom2D, breaking most of test_vispy_toolbar and also reported in glue-viz/glue#2311 Making the latest vispy version required for the latter; for echo there currently is no explicit dependency at all (probably pulled in from glue-core), but it's probably unlikely to run into a version < 0.2 now. _ This leaves two points of test failure:
  4. test_record reporting errors on writing to a closed file, not sure yet what to make of this:
    >       assert err.strip() == ""
    E       assert '--- Logging ...Arguments: ()' == ''
    E         + --- Logging error ---
    E         + Traceback (most recent call last):
    E         +   File "/Users/derek/opt/mambaforge/envs/py39/lib/python3.9/logging/__init__.py", line 1086, in emit
    E         +     stream.write(msg + self.terminator)
    E         +   File "/Users/derek/opt/mambaforge/envs/py39/lib/python3.9/site-packages/glue/app/qt/application.py", line 157, in write
    E         +     self._stderr_original.write(message)
    E         + ValueError: I/O operation on closed file....
  5. Isosurface viewer failure, leading down to a whole bunch of incompatibilities between vispy's VolumeVisual and what is currently implemented in MultiIsoVisual; putting this off to a separate PR.
codecov[bot] commented 2 years ago

Codecov Report

Merging #373 (1502b14) into master (9034cd3) will decrease coverage by 0.38%. The diff coverage is 42.50%.

@@            Coverage Diff             @@
##           master     #373      +/-   ##
==========================================
- Coverage   80.48%   80.10%   -0.39%     
==========================================
  Files          50       50              
  Lines        3885     4016     +131     
==========================================
+ Hits         3127     3217      +90     
- Misses        758      799      +41     
Impacted Files Coverage Δ
glue_vispy_viewers/compat/text.py 88.41% <ø> (+0.69%) :arrow_up:
...viewers/isosurface/tests/test_isosurface_viewer.py 37.31% <16.00%> (+6.16%) :arrow_up:
...e_vispy_viewers/common/tests/test_vispy_toolbar.py 25.26% <50.00%> (-57.50%) :arrow_down:
..._vispy_viewers/common/tests/test_3d_axis_visual.py 100.00% <100.00%> (ø)
...ue_vispy_viewers/common/tests/test_vispy_viewer.py 97.01% <100.00%> (+0.40%) :arrow_up:
...ue_vispy_viewers/common/tests/test_vispy_widget.py 100.00% <100.00%> (ø)
...vispy_viewers/scatter/tests/test_scatter_viewer.py 100.00% <100.00%> (ø)
glue_vispy_viewers/conftest.py 78.12% <0.00%> (-15.63%) :arrow_down:
glue_vispy_viewers/common/tools.py 59.49% <0.00%> (-11.40%) :arrow_down:
... and 21 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

dhomeier commented 2 years ago

@astrofrog I got the local tests under macOS/Python 3.9 down to 4 failed, 33 passed with this, so would like to get #371 merged and rebase to check if all the teardown errors with linux/macos here are due to the Azure setup.

dhomeier commented 2 years ago

Not pretty, but disabling the broken tests from (•4, •5) got rid of the teardown errors. Still should be checked against the Github CI as well.

dhomeier commented 2 years ago

@astrofrog some more issues came up after rebasing:

  1. Windows has no PyQt5 5.14 for py39 and a broken one for py310 (see last test failure here). Could move everything to 5.15 with all the xcb* deps, or just conditionally for sys_platform=='win32'.
  2. Windows runs are failing on (to me) obscure runtime errors

    .....tox\py38-test-dev\lib\site-packages\glue_vispy_viewers\scatter\tests\test_scatter_viewer.py s [ 45%] sWindows fatal exception: access violation Thread 0x000001c8 (most recent call first): File "D:\a\glue-Windows fatal exception: vaccess violationi spy-viewers\glue-vispy-viewers.tox\py38-test-dev\lib\site-pacERROR: InvocationError for command 'D:\a\glue-vispy-viewers\glue-vispy-viewers.tox\py38-test-dev\Scripts\pytest.EXE' --pyargs glue_vispy_viewers --cov glue_vispy_viewers '--cov-report=xml:D:\a\glue-vispy-viewers\glue-vispy-viewers\coverage.xml' (exited with code 3221225477)

They appeared actually already in #371, just missed them among the other failures. Also, the last Azure runs for this PR showed a bunch of exceptions/warnings like

RuntimeError: Using glBindFramebuffer with no OpenGL context. ValueError: I/O operation on closed file.

however still returned as "passed". I have skipped those tests on win32, but am now well into test_scatter_viewer, which did pass without any warnings in Azure, so there still seems to be something missing from the GH Actions setup on Windows. With no further backtrace, no idea what to do here except skipping possibly the entire rest of the tests for win32.

dhomeier commented 2 years ago

@astrofrog I was not able to restart the windows runs here to check if those problems are still present, but I think this should perhaps be merged to ensure at least vispy 0.11 support.