pylops
pylops copied to clipboard
Homogenize imports of optional libraries
Motivation
In PyLops, we have an increasing number of optional dependencies. For these dependencies 2 import patters have started to emerge:
- cupy-cusignal: via the
backend.py
module with availability check performed indeps.py
- others: direct import in files with try-except pattern
Proposed solution
- Add
*_enabled
variables for all other optional dependences indeps.py
- Add a function
*_import
for each optional dependences indeps.py
that parses error messages during imports of those optional dependencies. This function looks like this (eg for devito):
def devito_import(module):
if devito_enabled:
try:
import devito
except Exception as e:
devito_message = f"Failed to import devito (error:{e})."
else:
devito_message = (
f"Devito package not installed. In order to be able to use"
f'the {module} module run "pip install devito".'
)
return devito_message
and use it in combination with *_enabled
when import this library, eg:
if deps.devito_enabled:
import devito
from examples.seismic import AcquisitionGeometry, Model
from examples.seismic.acoustic import AcousticWaveSolver
else:
devito = None
devito_message = deps.devito_import("twoway")
instead of the old way
try:
import devito
from examples.seismic import AcquisitionGeometry, Model
from examples.seismic.acoustic import AcousticWaveSolver
except ModuleNotFoundError:
devito = None
devito_message = (
"Devito package not installed. In order to be able to use"
'the twoway module run "pip install devito".'
)
except Exception as e:
devito = None
devito_message = f"Failed to import devito (error:{e})."
So far the only downside I see for the new version is that a library gets imported twice instead ones. Is that a big deal?
Opinions?
Nice, I think this is definitely something we should tackle. With respect to the "double import", nothing to worry about since Python does not actually import it twice. It just skips it. To actually reimport a module one has to importlib.reload(themodule)
.
One thing, this pattern of try, import devito, otherwise devito = None triggers errors with mypy. One technique is described here, basically using global bools to keep track. So all in all I do agree with the design, but I would suggest skipping the devito = None
altogether and just checking if deps.devito_enabled
whever you want to check for it. Then one never accidentally tries to call a function of the optional module outside of a if deps.devito_enabled
block. What do you think?
Great, I wasn't aware about the double import thingy :)
Alright, good point about the None. I actually believe that in most places we never effectively do checks of the form if devito is not Note
, so the variable could be left emtpy. Let me try to make a PR following the code pattern I suggested for all optional dependencies and we can then finalize the discussion over there
This is currently not implemented in kirchhoff and lsm