8000 MAINT: use sysconfig not distutils.sysconfig where possible by mattip · Pull Request #17223 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content

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

Merged
merged 3 commits into from
Sep 4, 2020

Conversation

mattip
Copy link
Member
@mattip mattip commented Sep 2, 2020

Since python 3.2 sysconfig has moved to the stdlib. Some functionality is still needed from distutils.sysconfig, for instance distutils.sysconfig.customize_compiler and distutils.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.

@@ -187,6 +187,7 @@
import tempfile
import shutil

__all__ = ['system_info']
Copy link
Member Author

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
Copy link
Member Author

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',
Copy link
Member Author

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
Copy link
Member Author

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.

@eric-wieser
Copy link
Member
eric-wieser commented Sep 2, 2020

@@ -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,
Copy link
Member

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'

@mattip
Copy link
Member Author
mattip commented Sep 4, 2020

So this is now passing, and could be committed as-is. There is more to do:

  • distutils.sysconfig.customize_compiler has no counterpart in sysconfig, it probably should be a method of ccompiler clas.s
  • I think distutils.sysconfig.get_python_lib(standard_lib=1) could be replaced by sysconfig.get_path('data') but was not sure enough to put it into this PR
  • `distutils.sysconfig.get_config_var('EXT_SUFFIX') and sysconfig.get_config_var('EXT_SUFFIX') return different values on windows, is this a bug or intended behavior?

Edit: EXT_SUFFIX not what was previously

@mattip
Copy link
Member Author
mattip commented Sep 4, 2020

xref python/cpython#22088

@charris charris merged commit 02746c9 into numpy:master Sep 4, 2020
@charris
Copy link
Member
charris commented Sep 4, 2020

Thanks Matti.

tylerjereddy added a commit to tylerjereddy/numpy that referenced this pull request Sep 7, 2020
* 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
@mattip mattip deleted the setuptools1 branch April 8, 2021 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0