enthought / pyface

pyface: traits-capable windowing framework
Other
105 stars 55 forks source link

Consider making PythonEditor/PythonShell importable but fail later on init #800

Open kitchoi opened 3 years ago

kitchoi commented 3 years ago

Copied from this comment and this comment:

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

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")


The tradeoff here is that deferring the failure until one attempts to instantiate the editor might cause behaviours that are harder to debug (failing early is good). On the other hand, getting an ImportError from `from pyface.api import PythonEditor` could be just as bizarre. 
kitchoi commented 3 years ago

For the record, I prefer getting an ImportError on pygments when one attempts to import pyface.python_shell or pyface.python_editor directly when the dependency is missing. In other words, I oppose to the proposed change.

Unlike the api module, these modules have very narrow and specific scope for the feature they provide. If one needs Python Shell or Python editor in their GUI, then they need pygments (unless the editors have some fallback mechanism to show code without styled formatting, but that is a whole separate issue). Getting an import error early for missing pygments would make debugging easier for both users and maintainers.