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

Restores _default_compilers to ccompilers.py for backwards compatibility #346

Merged
merged 7 commits into from
Mar 20, 2025

Conversation

ManiacDC
Copy link
Contributor

@ManiacDC ManiacDC commented Mar 18, 2025

Additional changes for #336 and pypa/setuptools#4876

Adds back distutils.ccompiler._default_compilers for backwards compatibility

When importing the CC class from numba.pycc, numpy 1.23.5 will try to access distutils.ccompiler._default_compilers directly. This version still supports python 3.10, so it would be preferable not to deprecate this functionality until then.

@ManiacDC
Copy link
Contributor Author

I'm not sure if I updated the newsfragments correctly, specifically how to put a second bugfix on the same issue. Let me know if a new issue should have been opened.

@jaraco
Copy link
Member

jaraco commented Mar 19, 2025

[numpy 1.23.5] still supports python 3.10, so it would be preferable not to deprecate this functionality until then.

I'm not sure I'm following. Setuptools/distutils supports Python 3.9+. Following the bugfix/workaround for numba, what use-cases would benefit from this change? I'd really like to avoid carrying tech debt where it's not adding significant value.

@ManiacDC
Copy link
Contributor Author

[numpy 1.23.5] still supports python 3.10, so it would be preferable not to deprecate this functionality until then.

I'm not sure I'm following. Setuptools/distutils supports Python 3.9+. Following the bugfix/workaround for numba, what use-cases would benefit from this change? I'd really like to avoid carrying tech debt where it's not adding significant value.

I do understand your stance, I hate tech debt too. Any older packages using numpy.distutils (this is not limited to numba<0.57.0, there are a few hits that can be found via a quick google search) would no longer function without this attribute exported. I do not know how widespread these packages are. numpy.distutils is supported until python 3.11 is dropped.

@ManiacDC
Copy link
Contributor Author

This build failed again, I looked and failed to find how to set up my environment for distutils (packages needed for formatting, linting, and sorting). I think I found it now (I was looking in distutils, should have been looking in setuptools), I will try to address this tomorrow:

@jaraco
Copy link
Member

jaraco commented Mar 19, 2025

Distutils basically follows the skeleton process (tl;dr - run tox).

@jaraco
Copy link
Member

jaraco commented Mar 19, 2025

[numpy 1.23.5] still supports python 3.10, so it would be preferable not to deprecate this functionality until then.

I'm not sure I'm following. Setuptools/distutils supports Python 3.9+. Following the bugfix/workaround for numba, what use-cases would benefit from this change? I'd really like to avoid carrying tech debt where it's not adding significant value.

I do understand your stance, I hate tech debt too. Any older packages using numpy.distutils (this is not limited to numba<0.57.0, there are a few hits that can be found via a quick google search) would no longer function without this attribute exported. I do not know how widespread these packages are. numpy.distutils is supported until python 3.11 is dropped.

Sounds good. Once it's testing properly, I'll update it a bit to add comments to the imports to explain why they're there (and when they can be removed).

@m-gangloff
Copy link

m-gangloff commented Mar 19, 2025

[numpy 1.23.5] still supports python 3.10, so it would be preferable not to deprecate this functionality until then.

I'm not sure I'm following. Setuptools/distutils supports Python 3.9+. Following the bugfix/workaround for numba, what use-cases would benefit from this change? I'd really like to avoid carrying tech debt where it's not adding significant value.

I have the exact described issue with the faiss library when importing numpy.distutils.cpuinfo. I'm running on Python 3.10 and numpy 1.26.4 and setuptools 77.0.1.

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/opt/faiss/build/faiss/python/faiss/__init__.py", line 16, in <module>
    from .loader import *
  File "/opt/faiss/build/faiss/python/faiss/loader.py", line 88, in <module>
    instruction_sets = supported_instruction_sets()
  File "/opt/faiss/build/faiss/python/faiss/loader.py", line 53, in supported_instruction_sets
    if is_sve_supported():
  File "/opt/faiss/build/faiss/python/faiss/loader.py", line 43, in is_sve_supported
    import numpy.distutils.cpuinfo
  File "/usr/local/lib/python3.10/dist-packages/numpy/distutils/__init__.py", line 26, in <module>
    from . import ccompiler
  File "/usr/local/lib/python3.10/dist-packages/numpy/distutils/ccompiler.py", line 733, in <module>
    ccompiler._default_compilers += (('linux.*', 'intel'),
AttributeError: module 'distutils.ccompiler' has no attribute '_default_compilers'. Did you mean: 'get_default_compiler'?

* added documentation to imports as to why they are there, and when they can be removed.
@ManiacDC
Copy link
Contributor Author

@jaraco Thank you. I think I have it straightened out now (I couldn't get tox to run fully locally, probably due to improper setup, but the ruff checks pass now).
I also added some comments regarding why these imports were added and when they were removed. Feel free to edit/replace/keep since you said you were planning to add some.

@jaraco jaraco force-pushed the add-back-ccompiler._default_compilers branch from 82202f4 to b48a3b8 Compare March 20, 2025 01:46
@jaraco
Copy link
Member

jaraco commented Mar 20, 2025

I'm not committing to keeping this compatibility until Python 3.11 is dropped, but we can keep it for some time to alleviate the pressure. Are there versions of numpy that run on older Pythons without numpy.distutils?

@jaraco jaraco merged commit bcbad3a into pypa:main Mar 20, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants