enthought / mayavi

3D visualization of scientific data in Python
http://docs.enthought.com/mayavi/mayavi/
Other
1.3k stars 284 forks source link

Improve jupyter backends #1139

Closed prabhuramachandran closed 2 years ago

prabhuramachandran commented 2 years ago
prabhuramachandran commented 2 years ago

@corranwebster -- the only major change I am able to see between the headless test here and the one that was run on schedule from 5 days ago (https://github.com/enthought/mayavi/actions/runs/1937040765) is that traitsui was upgraded to 7.3.0 from 7.2.1. Could it be that simply importing traitsui.api triggers either a wxPython or Qt import before a configure_traits is invoked? This is possibly breaking the tests. I am just pushing a new commit to test this hypothesis. Indeed, that was it, so somehow traitsui 7.3.0 is importing the UI toolkit libraries. Is there a chance this can be fixed in a future traitsui release?

corranwebster commented 2 years ago

Could it be that simply importing traitsui.api triggers either a wxPython or Qt import before a configure_traits is invoked?

Unfortunately the Menu code in API causes the Pyface toolkit to be selected, so you will get a Qt or Wx import when you import traitsui.api. But I am thinking that there may be a regression in the way that Color and Font traits are defined.

corranwebster commented 2 years ago

Quick question: did the version of Pyface also change?

prabhuramachandran commented 2 years ago

@corranwebster -- no the earlier code was also on pyface-7.4.1.

corranwebster commented 2 years ago

When I try to replicate the failing test in a Python 3.9 virtualenv (not an EDM env) with no backends installed, I see the following line in the verbose output from doing from tvtk.api import tvtk; p = tvtk.Property():

# /Users/cwebster/p3.9/lib/python3.9/site-packages/pyface/ui/wx/__pycache__/__init__.cpython-39.pyc matches /Users/cwebster/p3.9/lib/python3.9/site-packages/pyface/ui/wx/__init__.py
# code object from '/Users/cwebster/p3.9/lib/python3.9/site-packages/pyface/ui/wx/__pycache__/__init__.cpython-39.pyc'
import 'pyface.ui.wx' # <_frozen_importlib_external.SourceFileLoader object at 0x132cd9bb0>
# /Users/cwebster/p3.9/lib/python3.9/site-packages/pyface/ui/wx/__pycache__/init.cpython-39.pyc matches /Users/cwebster/p3.9/lib/python3.9/site-packages/pyface/ui/wx/init.py
# code object from '/Users/cwebster/p3.9/lib/python3.9/site-packages/pyface/ui/wx/__pycache__/init.cpython-39.pyc'
# destroy pyface.ui.wx.init

and there are similar lines mentioning pyface.qt. However in this environment we can't be importing wx, because it isn't installed. But if I look at sys.modules['wx'] it isn't loaded and the selected toolkit is null as would be expected.

So I think that the failure is because some code is likely trying to load wx (and probably Qt) and failing, but things are showing up in the verbose logs that didn't before. The test probably needs to be refined to do something like inspecting sys.modules rather than just searching the logs.

prabhuramachandran commented 2 years ago

@corranwebster -- so sorry I sent you on a wild goose chase! I will look into writing a better and more robust test if that is possible. Thank you very much!

prabhuramachandran commented 2 years ago

@corranwebster -- I did some more investigation on this, while the test may not be ideal, it does do its job. It did detect what I think is a regression. If we just do from traitsui.view import View it imports a Qt or wx backend. To see where this actually happens I did some more digging:

  1. I see that the place this first starts is in the import from traitsui.ui import UI
  2. From here I looked at from traitsui.editor import Editor and I found that the culprit is the import from pyface.api import information in editor.py

I sent a simple PR to fix this (https://github.com/enthought/traitsui/pull/1883) and all it requires is delaying the import of information in the error method of the editor. This fixes the problem but you are very likely not going to like my test case at all.

prabhuramachandran commented 2 years ago

@rahulporuri -- can you please review this so I can merge it sooner rather than later. I can fix any issues related to the traitsui issue in a later PR.