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

BLD: Bump cibuildwheel and enable more PyPy #21285

Merged
merged 1 commit into from Apr 19, 2022
Merged

Conversation

lithomas1
Copy link
Collaborator

No description provided.

@github-actions github-actions bot added the 36 - Build Build related PR label Apr 2, 2022
@lithomas1
Copy link
Collaborator Author

Looks like tests are failing on pypy 3.9. cc @mattip.

 FAILED Users/runneradmin/AppData/Local/Temp/cibw-run-50vclubq/pp39-win_amd64/venv-test/lib/site-packages/numpy/distutils/tests/test_mingw32ccompiler.py::test_build_import
  FAILED Users/runneradmin/AppData/Local/Temp/cibw-run-50vclubq/pp39-win_amd64/venv-test/lib/site-packages/numpy/typing/tests/test_generic_alias.py::TestGenericAlias::test_pass[__repr__-<lambda>]
  FAILED Users/runneradmin/AppData/Local/Temp/cibw-run-50vclubq/pp39-win_amd64/venv-test/lib/site-packages/numpy/typing/tests/test_generic_alias.py::TestGenericAlias::test_pass[__dir__-<lambda>]
  FAILED Users/runneradmin/AppData/Local/Temp/cibw-run-50vclubq/pp39-win_amd64/venv-test/lib/site-packages/numpy/typing/tests/test_generic_alias.py::TestGenericAlias::test_raise[setattr-AttributeError-<lambda>0]
  FAILED Users/runneradmin/AppData/Local/Temp/cibw-run-50vclubq/pp39-win_amd64/venv-test/lib/site-packages/numpy/typing/tests/test_generic_alias.py::TestGenericAlias::test_raise[setattr-AttributeError-<lambda>1]
  = 5 failed, 18854 passed, 890 skipped, 33 xfailed, 30 xpassed, 1 warning in 4876.94s (1:21:16) =

(Sorry that I couldn't get the full log. My machine is lagging quite a bit with the verbose logs).

@mattip
Copy link
Member

mattip commented Apr 3, 2022

My machine is lagging quite a bit with the verbose logs

I usually click on the gear and then download the raw logs since scrolling to the end of the formatted log seems impossible.

As for the failures: is there a way to convince cibuildwheel to use PyPy3.9 v7.3.9 and not PyPy3.9 v7.3.8 (which was the version mentioned in the PR to add PyPy3.9 ?

@mattip
Copy link
Member

mattip commented Apr 3, 2022

Maybe add a line to print the python version python -c "import sys; print(sys.version)" so we can see what exact python is being used.

@mattip
Copy link
Member

mattip commented Apr 3, 2022

Ahh, nvrmind. I see the docker image is quay.io/pypa/manylinux2014_x86_64:2022-03-31-361e6b6, and it is using Python 3.9.12[pypy-7.3.9-final]. This is strange because there are weekly tests of PyPy against NumPy HEAD and they are passing.

@mattip
Copy link
Member

mattip commented Apr 5, 2022

Adding logic to skip the tests on the bad PyPy versions makes CI pass.

@charris charris closed this Apr 6, 2022
@charris charris reopened this Apr 6, 2022
@lithomas1
Copy link
Collaborator Author

@mattip Just to confirm, the tests that are skipped here will not be skipped on PyPy's nightly testing, right?

@lithomas1
Copy link
Collaborator Author

Adding logic to skip the tests on the bad PyPy versions makes CI pass.

Looks like the fixes cause other tests to fail, now, unfortunately.

I think it might be better to just skip pp39 wheel builds for now. PyPy 3.9 is in beta right now, and if the tests are going to be fixed in the next release of PyPy(which hopefully also makes pp39 stable), I'd rather wait until then. I believe PyPy tests against the numpy build suite, so its unlikely more/other tests will fail.

@mattip
Copy link
Member

mattip commented Apr 13, 2022

I think it might be better to just skip pp39 wheel builds for now

Sounds good.

@mattip
Copy link
Member

mattip commented Apr 14, 2022

FWIW, NumPy HEAD + PyPy 3.9 HEAD is passing the full test suite after I extended the timeout to over 70 minutes for macOS, so NumPy may want to consider adding PyPy39 after the next PyPy release. So I think once ppy39 is removed from this PR it should be ready.

We should have a reminder somewhere to make sure we are using vs2019 v14.1 and not v14.2.

@mattip
Copy link
Member

mattip commented Apr 14, 2022

We should have a reminder somewhere to make sure we are using vs2019 v14.1 and not v14.2.

This seems critical since using 14.2 will break SciPy and anyone else using npmath.lib on windows. xref #20880

@h-vetinari
Copy link
Contributor

This seems critical since using 14.2 will break SciPy and anyone else using npmath.lib on windows

The word "critical" is way too strong here IMO. The breakage is restricted to those using VS2017, an EOL compiler, for which free and ABI-compatible replacements are available (VS2019, VS2022).

I have trouble imagining a scenario where someone cannot do an ABI-compatible upgrade, and I have yet to hear of someone affected in this way (I made the same case in conda-forge an no-one came forward so far). There's IMO a substantial possibility that we're jumping through hoops for an almost empty set of users here (and at some point, if one insists on using EOL software, it should be completely unsurprising that eventually one does not get to use the newest library versions anymore).

Perhaps unsurprisingly (given the above), I think VS2017 support should simply be dropped.

@mattip
Copy link
Member

mattip commented Apr 15, 2022

The problem here is users with fortran code. The best option for a fortran compiler is mingw64, which cannot link with the npymath.lib created from vs2019+.

@h-vetinari
Copy link
Contributor

The best option for a fortran compiler is mingw64, which cannot link with the npymath.lib created from vs2019+.

Ah, thanks, I was not aware of that restriction. Then I retract the suggestion to drop VS2017 (is there any issue to follow for mingw support of VS2019? Because VS2017 has been removed from all MSFT-owned CI providers already, and is very much on its way out).

@mattip
Copy link
Member

mattip commented Apr 15, 2022

One place to start is in MacPython/numpy-wheels#145, which has links to both the Visual Studio response to the problem (basically: yes it is a known thing, we will document the breaking change), and a repo set up to investigate the problem matthew-brett/dll_investigation#1. The latter has a link to the mingw64 discussion

Mixing static libraries or object files between mingw and msvc is not
supported, and not expected to work, in general. You might have been lucky
and your library might have been simple enough not to hit any problematic
case though.

There is also issue #20880 to change from static linking to dynamic linking (ship a npymath.dll), which would mean we don't need the 14.1 toolchain.

@lithomas1 lithomas1 marked this pull request as ready for review April 18, 2022 16:48
@lithomas1 lithomas1 requested a review from mattip April 18, 2022 22:13
@mattip mattip merged commit 55aacc7 into numpy:main Apr 19, 2022
@mattip
Copy link
Member

mattip commented Apr 19, 2022

Thanks @lithomas1. The follow up to the 14.1/14.2 toolchain issues is #20880

@lithomas1 lithomas1 deleted the patch-3 branch April 19, 2022 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
36 - Build Build related PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants