Skip to content
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

BUG: importing numpy.distutils replaces the global logger with a custom class which doesn't support kwargs #24213

Closed
oliver-s-lee opened this issue Jul 19, 2023 · 6 comments
Labels
00 - Bug 57 - Close? Issues which may be closable unless discussion continued component: numpy.distutils

Comments

@oliver-s-lee
Copy link

oliver-s-lee commented Jul 19, 2023

Describe the issue:

Importing numpy.distutils silently replaces the global logger returned by logging.getLogger() with the custom class from numpy.distutils.log. This is surprising in of itself, but additionally the signature of numpy.distutils.log.Log._log() is outdated/incorrect and does not support the kwargs exc_info, stack_info, stacklevel or extra. In particular, this means the exception() method of the custom logger does not work.

Reproduce the code example:

>>> import logging
>>> type(logging.getLogger())
<class 'logging.RootLogger'>
>>> import numpy.distutils
>>> type(logging.getLogger())
<class 'numpy.distutils.log.Log'>
>>> logging.getLogger().exception("Oh No")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/oliver/anaconda3/envs/python3.10.12/lib/python3.10/logging/__init__.py", line 1512, in exception
    self.error(msg, *args, exc_info=exc_info, **kwargs)
  File "/home/oliver/anaconda3/envs/python3.10.12/lib/python3.10/logging/__init__.py", line 1506, in error
    self._log(ERROR, msg, args, **kwargs)
TypeError: Log._log() got an unexpected keyword argument 'exc_info'

Error message:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/oliver/anaconda3/envs/python3.10.12/lib/python3.10/logging/__init__.py", line 1512, in exception
    self.error(msg, *args, exc_info=exc_info, **kwargs)
  File "/home/oliver/anaconda3/envs/python3.10.12/lib/python3.10/logging/__init__.py", line 1506, in error
    self._log(ERROR, msg, args, **kwargs)
TypeError: Log._log() got an unexpected keyword argument 'exc_info'

Runtime information:

>>> import sys, numpy; print(numpy.__version__); print(sys.version) 
1.22.3
3.10.8 (main, Nov 24 2022, 14:13:03) [GCC 11.2.0]

Context for the issue:

Any package that imports anything from numpy.distutils will accidentally break the root logger for the whole program/script. In addition, because it's not obvious that the root logger is replaced in this way, this problem can be (and sadly was) challenging to diagnose.

For a real world example, importing pyscf also recreates this behaviour:

>>> import logging
>>> type(logging.getLogger())
<class 'logging.RootLogger'>
>>> import pyscf
>>> type(logging.getLogger())
<class 'numpy.distutils.log.Log'>
>>> try:
...     raise Exception("Bugger")
... except Exception:
...     logging.getLogger().exception("Oh No")
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
Exception: Bugger

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 4, in <module>
  File "/home/oliver/anaconda3/envs/python3.10.12/lib/python3.10/logging/__init__.py", line 1512, in exception
    self.error(msg, *args, exc_info=exc_info, **kwargs)
  File "/home/oliver/anaconda3/envs/python3.10.12/lib/python3.10/logging/__init__.py", line 1506, in error
    self._log(ERROR, msg, args, **kwargs)
TypeError: Log._log() got an unexpected keyword argument 'exc_info'
@charris
Copy link
Member

charris commented Jul 19, 2023

I am going to close this. Distutils is very old and now deprecated, we are moving to meson for NumPy 2.0.0 and 1.26.0 builds. Python itself dropped distutils in Python 3.12.

If you have questions about the transition, feel free to continue the discussion here.

@charris charris closed this as completed Jul 19, 2023
@oliver-s-lee
Copy link
Author

Fair enough. For completeness and for other's reference, I should point out that this does not only affect importing numpy.distutils directly, but also crops up in other places where numpy itself imports from numpy.distutils. For example, numpy.ctypeslib.load_library() uses numpy.distutils.misc_util.get_shared_lib_extension, and hence calling load_library() will break your root logger.

@charris
Copy link
Member

charris commented Jul 20, 2023

I should point out that this does not only affect importing numpy.distutils directly,

Is this in the main branch, or in a release? If this is still in the main branch, it will need fixing. You might try one of the nightly wheels to see if there are still problems: https://anaconda.org/scientific-python-nightly-wheels/numpy/ .

@charris charris reopened this Jul 20, 2023
@mattip
Copy link
Member

mattip commented Jul 21, 2023

The main branch still uses numpy.distutils.misc_util.get_shared_lib_extension, which is still needed since sysconfig.get_config_vars('SHLIB_SUFFIX') still returns .so on darwin, which is wrong. It should be .dylib. So we should move that function out of distutils (maybe copy it into ctypeslib.py).

At some point after finishing #24206 (which allows removing setuptools from the test_requirements.txt pip requirements file), we should try deleting numpy.distutils to see what tests will fail because we still use it internally.

@rgommers
Copy link
Member

The main branch still uses numpy.distutils.misc_util.get_shared_lib_extension, which is still needed since sysconfig.get_config_vars('SHLIB_SUFFIX') still returns .so on darwin, which is wrong. It should be .dylib. So we should move that function out of distutils (maybe copy it into ctypeslib.py).

It doesn't:

numpy/numpy/ctypeslib.py

Lines 131 to 142 in b0ff5c5

# Try to load library with platform-specific name, otherwise
# default to libname.[so|dll|dylib]. Sometimes, these files are
# built erroneously on non-linux platforms.
base_ext = ".so"
if sys.platform.startswith("darwin"):
base_ext = ".dylib"
elif sys.platform.startswith("win"):
base_ext = ".dll"
libname_ext = [libname + base_ext]
so_ext = sysconfig.get_config_var("EXT_SUFFIX")
if not so_ext == base_ext:
libname_ext.insert(0, libname + so_ext)
. Also the 1.25.x branch is clean.

numpy.distutils is not installed at all on Python 3.12, and all tests pass there. So any functionality in NumPy that has tests no longer relies on numpy.distutils. It's possible that something untested slipped through the cracks, but that'll be pretty niche and likely will not be found by manually deleting the folder.

I searched the repo again, it's a bit difficult to do because of the many comments, setup.py files etc. where distutils and numpy.distutils are still mentioned/used. But I don't see anything really problematic.

I agree with closing this issue, because of the orginal reason given: numpy.disutils is deprecated and slated to disappear, so we don't really want to spend time on this kind of thing anymore.

@rgommers rgommers added component: numpy.distutils 57 - Close? Issues which may be closable unless discussion continued labels Jul 22, 2023
@mattip
Copy link
Member

mattip commented Jul 23, 2023

It doesn't:
numpy.distutils is not installed at all on Python 3.12, and all tests pass there.

Got, thanks. I must have been looking at a different branch. Thanks for re-checking. Closing.

@mattip mattip closed this as completed Jul 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
00 - Bug 57 - Close? Issues which may be closable unless discussion continued component: numpy.distutils
Projects
None yet
Development

No branches or pull requests

4 participants