-
Notifications
You must be signed in to change notification settings - Fork 27
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
pyproject.toml, setup.py: Only use numpy.distutils on win32 #112
pyproject.toml, setup.py: Only use numpy.distutils on win32 #112
Conversation
setup.py
Outdated
@@ -6,7 +6,7 @@ | |||
from Cython.Build import cythonize | |||
|
|||
|
|||
if sys.version_info < (3, 12): | |||
if sys.platform == 'win32': |
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.
We need < 3.12 on Windows as well because the numpy.distutils module is removed in Python 3.12. I guess it could be:
if sys.platform == 'win32' and sys.version_info < (3, 12)
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.
but does the case sys.platform == 'win32'
and sys.version_info >= (3, 12)
work at all?
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.
Yes, it works. Every CI job normally passes but the change here has caused the Windows job to fail when building wheels for Python 3.12. These same CI jobs are what build the wheels that go to PyPI which has Windows wheels for Python 3.12:
https://pypi.org/project/python-flint/#files
Some sort of change in CPython itself means that mingw+setuptools works for Python 3.12 but not for Python < 3.12. I don't follow all the details but the clearest explanation I found is here:
cython/cython#4470 (comment)
a23aae5
to
54a049e
Compare
Thanks. I've updated it accordingly |
Looks like its working. Thanks Matthias. |
The Windows build with cibuildwheel is now failing CI in gh-113. I'm not sure if this or other changes is the cause. |
https://github.com/flintlib/python-flint/actions/runs/7748025255/job/21130728629?pr=113#step:5:81 |
I don't see why that error should be thrown now and only on Windows. The CI passes after merging this PR: I've just restarted that same CI it to see if it passes again and it has now failed. The cibuildwheel version does not seem to have changed. The msys version does not seem to have changed. The Python versions have not changed. The same argument |
Maybe an upstream bug: |
I think it is working after bumping the version of cibuildwheel to 2.16.5. Apparently the problem was a change in the GitHub Actions runners that needed an update in the pypa/cibuildwheel action. |
All CI now passed so that looks to be the fix. Thanks for looking at this @mkoeppe |
Using
setuptools
on all other platforms, as is already done when Python == 3.12 after #100.Ref #52 (comment) @oscarbenjamin