-
-
Notifications
You must be signed in to change notification settings - Fork 10.9k
MAINT: use sysconfig not distutils.sysconfig where possible #17223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -187,6 +187,7 @@ | |||
import tempfile | |||
import shutil | |||
|
|||
__all__ = ['system_info'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file was missing an __all__
, so it was re-exporting imported modules.
@@ -26,7 +26,8 @@ def UnixCCompiler__compile(self, obj, src, ext, cc_args, extra_postargs, pp_opts | |||
self.compiler_so = ccomp | |||
# ensure OPT environment variable is read | |||
if 'OPT' in os.environ: | |||
from distutils.sysconfig import get_config_vars | |||
# XXX who uses this? | |||
from sysconfig import get_config_vars |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When is OPT
ever used?
'numpy.distutils.system_info.sys', | ||
'numpy.distutils.system_info.tempfile', | ||
'numpy.distutils.system_info.textwrap', | ||
'numpy.distutils.system_info.warnings', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that system_info
has an __all__
, these can be removed.
@@ -445,7 +445,7 @@ def build_project(args): | |||
os.makedirs(site_dir) | |||
if not os.path.exists(site_dir_noarch): | |||
os.makedirs(site_dir_noarch) | |||
env['PYTHONPATH'] = site_dir + ':' + site_dir_noarch | |||
env['PYTHONPATH'] = site_dir + os.pathsep + site_dir_noarch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This mistake makes me suspicious - do we really need to set PYTHONPATH
here? It is done correctly before running tests.
For reference: |
numpy/distutils/system_info.py
Outdated
@@ -255,7 +256,7 @@ def libpaths(paths, bits): | |||
|
|||
if sys.platform == 'win32': | |||
default_lib_dirs = ['C:\\', | |||
os.path.join(distutils.sysconfig.EXEC_PREFIX, | |||
os.path.join(sysconfig.EXEC_PREFIX, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AttributeError: module 'sysconfig' has no attribute 'EXEC_PREFIX'
So this is now passing, and could be committed as-is. There is more to do:
Edit: |
xref python/cpython#22088 |
Thanks Matti. |
* numpygh-17223 started the `distutils` modernization process because of the various upstream stuff going on (thanks) * that set of changes caused build issues with NumPy pre-release wheels + SciPy `master`: https://travis-ci.org/github/scipy/scipy/builds/725002838 * I was able to patch two separate issues and confirm that this feature branch restores the SciPy build locally: 1) in one case, `distutils.sysconfig.get_python_inc` was replaced with `distutils.get_python_inc`, which does not exist (probably a typo) 2) a bit more serious; even `sysconfig.get_python_inc` does not appear to exist; the closest match I could find (and that worked) was `sysconfig.get_path()` with options for general and platform-specific header paths as needed * I'm not claiming that these stdlib replacements are perfect either (the prefix case may still be slightly different?), but it does prevent the SciPy build break originally introduced as a start
Since python 3.2
sysconfig
has moved to the stdlib. Some functionality is still needed fromdistutils.sysconfig
, for instancedistutils.sysconfig.customize_compiler
anddistutils.sysconfig.get_python.lib
.It is not clear if this is a (very small) step toward fixing the distutils/setuptools mess, but it seems like the right thing to do anyway.