BLD lazy import of tempita to avoid cython dependency #15313
Conversation
sklearn/utils/setup.py
Outdated
| @@ -58,6 +57,7 @@ def configuration(parent_package='', top_path=None): | |||
|
|
|||
| with open(pyxfiles, "r") as f: | |||
| tmpl = f.read() | |||
| from Cython import Tempita # noqa | |||
There was a problem hiding this comment.
Could this be a function in sklearn/_build_utils/__init__.py. Something along these lines:
def tempita_sub(*args, **kwargs):
"""Helper function.
This does lazy import of Cython to make sure that we do not depend on Cython when building from a release sdist (since it contains the .c files already)
"""
from Cython import Tempita
Tempita.sub(*args, **kwargs)There was a problem hiding this comment.
Or adding a comment above the import about it.
sklearn/utils/setup.py
Outdated
| @@ -58,6 +57,7 @@ def configuration(parent_package='', top_path=None): | |||
|
|
|||
| with open(pyxfiles, "r") as f: | |||
| tmpl = f.read() | |||
| from Cython import Tempita # noqa | |||
There was a problem hiding this comment.
Or adding a comment above the import about it.
|
I went for the utility fonction |
| import numpy | ||
|
|
||
| from sklearn._build_utils import gen_from_templates |
There was a problem hiding this comment.
out of curiosity, can you explain me why an absolute import is required here ?
There was a problem hiding this comment.
It doesn't work if we use a relative import?
There was a problem hiding this comment.
No, they also did it for the deprecated modules in sklearn/setup.py
Maybe @thomasjpfan knows better about that ?
There was a problem hiding this comment.
Thanks @jeremiedbb , I think it's clear now.
LGTM, though I'm still curious about the need for the absolute import
Fixes #15298
Same fix as #13980.
There are no other occurrence.