Improve the way to import optional modules #2809
Conversation
c52fecf to
b1c40b0
Compare
pygmt/tests/test_figure.py
Outdated
| try: | ||
| import IPython | ||
| except ImportError: | ||
| IPython = None # pylint: disable=invalid-name |
There was a problem hiding this comment.
Another option looking at https://stackoverflow.com/questions/61384752/how-to-type-hint-with-an-optional-import/77341843#77341843 seems to be something like:
IPython: Any = None
try:
import IPython
except ImportError:
passThough not sure if this is recommended.
There was a problem hiding this comment.
It's unclear what the recommended way is for importing an optional module. It seems sometimes pandas and xarray use the has_xxx bool variable:
but sometimes they also use the old way, which mypy should complain, e.g.,
https://github.com/pydata/xarray/blob/49bd63a8332c1930a866724a2968b2d880dae25e/xarray/coding/times.py#L31
There was a problem hiding this comment.
Looking at mypy (e.g. python/mypy#1297 (comment)), they also seem to recommend a try-except-else pattern? Something like:
try:
import IPython as _IPython
except ImportError:
IPython = None
else:
IPython = _IPythonThere was a problem hiding this comment.
I don't think it's officially recommended and it's also more difficult to understand than IPython = None and _HAS_IPYTHON = False.
There was a problem hiding this comment.
Ok, let's go with _HAS_IPython unless there's a better recommended way 🙂
That said, we should probably decide first if adding mypy is something we want (#2808), otherwise the changes here aren't really needed.
For optional modules, currently we do like this:
but Mypy doesn't like it:
In this PR, we use another way to import optional modules:
See https://adamj.eu/tech/2021/12/29/python-type-hints-optional-imports/ for more detailed explanations.
Sometimes, modules like
IPythonare not directly used, then ruff reports F401 and recommends usingimportlib.util.find_spec.Changes in this PR: