Make pygments optional for pyface.api

[kitchoi]

Closes #655

Depending on the project context, PythonEditor and PythonShell may never be needed, and yet importing pyface.api still requires pygments. This PR modifies pyface.api such that if import of PythonEditor and PythonShell fail because of missing pygments, it will carry on. It makes maintaining dependencies easier for projects that don't use PythonEditor/PythonShell.

This PR does not modify the extras_requires in setup.py. Pip-installing pyface with the Qt tag is still going to bring in pygments, see discussion in #655.

[codecov-io]

Codecov Report

Merging #796 (1102fbe) into master (4bccc3e) will increase coverage by 0.21%.
The diff coverage is 71.42%.

@@            Coverage Diff             @@
##           master     #796      +/-   ##
==========================================
+ Coverage   40.67%   40.88%   +0.21%     
==========================================
  Files         508      508              
  Lines       27832    27837       +5     
  Branches     4217     4217              
==========================================
+ Hits        11320    11382      +62     
+ Misses      16016    15981      -35     
+ Partials      496      474      -22     

Impacted Files	Coverage Δ
pyface/api.py	95.65% <71.42%> (-4.35%)	⬇️
pyface/ui/qt4/tasks/split_editor_area_pane.py	65.41% <0.00%> (+0.59%)	⬆️
pyface/ui/qt4/console/console_widget.py	29.28% <0.00%> (+0.72%)	⬆️
pyface/ui/qt4/tasks/advanced_editor_area_pane.py	51.72% <0.00%> (+0.86%)	⬆️
pyface/wx/python_stc.py	10.36% <0.00%> (+1.21%)	⬆️
pyface/ui/qt4/tasks/task_window_backend.py	60.52% <0.00%> (+1.31%)	⬆️
pyface/ui/qt4/code_editor/code_widget.py	44.39% <0.00%> (+1.69%)	⬆️
pyface/ui/qt4/tasks/editor_area_pane.py	55.88% <0.00%> (+2.20%)	⬆️
pyface/ui/qt4/application_window.py	91.66% <0.00%> (+2.38%)	⬆️
pyface/ui/qt4/util/gui_test_assistant.py	80.53% <0.00%> (+2.65%)	⬆️
... and 9 more

---

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 4bccc3e...db95182. Read the comment docs.

[kitchoi]

I verified this test failed when I have an environment with Qt but no pygments. Then the test passes after I made the change in api.py.

Ideally our CI matrix should cover more minimal setups...

[corranwebster]

This will block usage of the PythonEditor and the PythonShell with wx if Pygments is not installed, even though the Wx backend doesn't use Pygments.

I think if you move this import:

pyface/pyface/ui/qt4/code_editor/code_widget.py

Line 22 in 4bccc3e

from .pygments_highlighter import PygmentsHighlighter

to here:

pyface/pyface/ui/qt4/code_editor/code_widget.py

Line 39 in 4bccc3e

self.highlighter = PygmentsHighlighter(self.document(), lexer)

And potentially, if the import fails we can just continue on with highlighter set to the default, which means graceful degradation of capabilities.

Something similar would be needed for the shell, with a slight complication that it interacts more with the highlighter.

There may be alternative approaches that work, but it feels like we want to handle this in the qt-specific code.

[corranwebster]

This change might also cause problems for TraitsUI because:

• the wx CodeEditor imports from pyface.api so it will fail it import time if Pygments is not installed, but Pygments isn't a requirement: https://github.com/enthought/traitsui/blob/01db38d6bce242de1426b279d38d5025fcbbfc32/traitsui/wx/code_editor.py#L35
• the qt CodeEditor directly imports from the Pyface internals, so it will still fail without Pygments: https://github.com/enthought/traitsui/blob/01db38d6bce242de1426b279d38d5025fcbbfc32/traitsui/qt4/code_editor.py#L20
• the generic ShellEditor will still fail at instantiation time because it doesn't import from pyface.api: https://github.com/enthought/traitsui/blob/148457a08b4f0fc14ccff6dc2166fa8f0251643c/traitsui/editors/shell_editor.py#L51

[kitchoi]

The concern for wx CodeEditor is legit.

the qt CodeEditor directly imports from the Pyface internals, so it will still fail without Pygments:
the generic ShellEditor will still fail at instantiation time because it doesn't import from pyface.api:

I think when one does need the Qt implementation of CodeEditor, then they need to have pygments. The goal here is not to make CodeEditor work without pygments. That could be a separate issue, though.

[kitchoi]

@corranwebster I looked into the Qt specific code you mentioned, and I find that somewhat messy. While moving the import down in PythonShell could defer the import error, it defers the error all the way to when the class is instantiated. The PythonShell, like you said, is even trickier because it is subclassing. And even if I try to make that work, the final solution does not look like it would last: It is too easy to add an import in the Qt specific modules that could reintroduce the dependency.

Taking a step back, at the moment, pygments is still a core dependency for PythonShell and PythonEditor, so failing early when they are imported seems fair. What we are trying to achieve is to make api.py more flexible.

I attempted the other approach which only touches api.py, which would preserve the import of PythonShell and PythonEditor for wx, and added more tests to ensure that.

[kitchoi]

#776 strikes again! :grumpy:

[corranwebster]

I'm OK with this implementation - it solves the immediate problem - however I think we will end up with different error reports from users about PythonEditor and PythonShell not importing correctly. Time will tell if this is better or worse than the current situation.

[corranwebster]

I have an alternative suggestion:

• do the import error check in pyface.python_shell and pyface.python_editor
• if it fails, return an object which fails on instantiation with a message about needing pygments

[corranwebster]

Something like:

from .toolkit import toolkit_object

try:
    PythonEditor = toolkit_object("python_editor:PythonEditor")
except ImportError as _exception:
    # Excuse pygments dependency (for Qt), otherwise re-raise
    if _exception.name != "pygments":
        raise
    def PythonEditor(**traits):
        raise RuntimeError("PythonEditor on Qt requires Pygments to be installed")

[kitchoi]

Thanks @corranwebster

I have an alternative suggestion:

do the import error check in pyface.python_shell and pyface.python_editor
if it fails, return an object which fails on instantiation with a message about needing pygments

Shall I open a separate issue for this implementation option and merge this one?

[kitchoi]

I also thought about raising a warning in the api.py but I thought that might be too noisy... We could however log at the debug level where we capture and silence the ImportError, which should be invisible by default. What do you think about that?

[kitchoi]

I will merge this now. I opened #800 for considering the alternative approach in the future (subject to feedback). I will open a separate PR on the logging idea.

[kitchoi]

For reference, the decision for not emitting a warning is partly similar to enthought/traitsui#731. In TraitsUI, ArrayEditor is not available when NumPy is not available. While traitsui.api does not warn, traitsui.editors.api does. When one does not need the ArrayEditor, the import warning is just noise.