serge-sans-paille / pythran

Ahead of Time compiler for numeric kernels
https://pythran.readthedocs.io
BSD 3-Clause "New" or "Revised" License
2.01k stars 193 forks source link

Recursive import issue with Pythran when building SciPy #2126

Closed rgommers closed 1 year ago

rgommers commented 1 year ago

We're hitting a recursive import error when building SciPy twice with pip install -e . --no-build-isolation. The error happens only for editable installs of SciPy, however the recursion is also there with normal builds. The SciPy build has to do:

import pythran
pythran.get_include()

in its build script to obtain Pythran's include directory. However, that import statement basically pulls in all of the Pythran code base, and this errors out at:

pythran/tables.py", line 4565, in save_arguments
    themodule = import_module(".".join(module_name))

What is happening there is that that code fails when module_name equals 'scipy.special', creating an import loop.

Full traceback (from https://github.com/scipy/scipy/issues/18900):

``` Traceback (most recent call last): File "", line 3, in File "/home/rgommers/mambaforge/envs/scipy-dev/lib/python3.10/site-packages/pythran/__init__.py", line 50, in from pythran.toolchain import (generate_cxx, compile_cxxfile, compile_cxxcode, File "/home/rgommers/mambaforge/envs/scipy-dev/lib/python3.10/site-packages/pythran/toolchain.py", line 6, in from pythran.backend import Cxx, Python File "/home/rgommers/mambaforge/envs/scipy-dev/lib/python3.10/site-packages/pythran/backend.py", line 7, in from pythran.analyses import LocalNodeDeclarations, GlobalDeclarations, Scope File "/home/rgommers/mambaforge/envs/scipy-dev/lib/python3.10/site-packages/pythran/analyses/__init__.py", line 12, in from .aliases import Aliases, StrictAliases File "/home/rgommers/mambaforge/envs/scipy-dev/lib/python3.10/site-packages/pythran/analyses/aliases.py", line 6, in from pythran.tables import functions, methods, MODULES File "/home/rgommers/mambaforge/envs/scipy-dev/lib/python3.10/site-packages/pythran/tables.py", line 4598, in save_arguments((), MODULES) File "/home/rgommers/mambaforge/envs/scipy-dev/lib/python3.10/site-packages/pythran/tables.py", line 4561, in save_arguments save_arguments(module_name + (elem,), signature) File "/home/rgommers/mambaforge/envs/scipy-dev/lib/python3.10/site-packages/pythran/tables.py", line 4561, in save_arguments save_arguments(module_name + (elem,), signature) File "/home/rgommers/mambaforge/envs/scipy-dev/lib/python3.10/site-packages/pythran/tables.py", line 4565, in save_arguments themodule = import_module(".".join(module_name)) File "/home/rgommers/mambaforge/envs/scipy-dev/lib/python3.10/importlib/__init__.py", line 126, in import_module return _bootstrap._gcd_import(name[level:], package, level) File "", line 1050, in _gcd_import File "", line 1027, in _find_and_load File "", line 1002, in _find_and_load_unlocked File "", line 945, in _find_spec File "/home/rgommers/mambaforge/envs/scipy-dev/lib/python3.10/site-packages/_scipy_editable_loader.py", line 268, in find_spec tree = self.rebuild() File "/home/rgommers/mambaforge/envs/scipy-dev/lib/python3.10/site-packages/_scipy_editable_loader.py", line 309, in rebuild subprocess.run(self._build_cmd, cwd=self._build_path, env=env, stdout=stdout, check=True) File "/home/rgommers/mambaforge/envs/scipy-dev/lib/python3.10/subprocess.py", line 526, in run raise CalledProcessError(retcode, process.args, subprocess.CalledProcessError: Command '['/home/rgommers/mambaforge/envs/scipy-dev/bin/ninja']' returned non-zero exit status 1. ../../scipy/meson.build:92:21: ERROR: Command `/home/rgommers/mambaforge/envs/scipy-dev/bin/python3.10 -c 'import os os.chdir(os.path.join("..", "tools")) import pythran try: incdir = os.path.relpath(pythran.get_include()) except Exception: incdir = pythran.get_include() print(incdir) '` failed with status 1. ```

To make matters worse, it's not so easy to work around it, because disabling pythran in the scipy build is not helping - Cython contains an import pythan statement too in a try-except loop.

My suggested solution here is to make the imports in pythran/__init__.py lazy via the use of __getattr__. That will solve both of the above issues at once, and I suspect it may also help with robustness for Cython in general (it only accesses pythran.__version__ if --np-pythran isn't explicitly used). If that sounds fine to you @serge-sans-paille, then I'd be happy to submit a PR.

serge-sans-paille commented 1 year ago

Pythran should behave correctly is scipy is not importable - that's a bug. We should just skip trying to model scipy.special builtins if they are not importable.

serge-sans-paille commented 1 year ago

Mmmh looks like we already have a try-except catch here, but it doesn't catch the expected exception...

rgommers commented 1 year ago

I don't think any try-except will do the right thing unfortunately, since import scipy will hit an import hook from the importlib machinery (https://github.com/mesonbuild/meson-python/blob/main/mesonpy/_editable.py#L297) and that will not be happy. I really need to avoid calling import scipy during the build of scipy itself. Supporting editable (in-place) builds with an out-of-place build system is inherently a little obscure unfortunate.

That said, I've seen Cython crash before with Pythran installed but not requested for use, so the problem isn't limited to SciPy's editable builds. Calling this much code on a plain import pythran is not very robust - an import cycle is bad even within a try-except.

serge-sans-paille commented 1 year ago

oh, thanks for explaining. I'm convinced and as you offered to provide a PR for lazy loading, I cannot say no ;-)

rgommers commented 1 year ago

Thanks! I'll work on this soon.

serge-sans-paille commented 1 year ago

@rgommers : the definition of soon as expired :-) Do you want me to give it a try?

rgommers commented 1 year ago

Sorry, it took me way longer than expected to finally finish the build system stuff for Python 3.12 - we only got a numpy beta release out yesterday.

I should still be able to do this, but probably not this week - if you're in a hurry maybe you want to give it a try?

serge-sans-paille commented 1 year ago

I'm definitively in a hurry, and if you're not either, let's take our time :-)

serge-sans-paille commented 1 year ago

@rgommers I'll do a release once you've fixed this one, that should help with main numpy / scipy builds

rgommers commented 1 year ago

Thanks. I'll have a go at this today.