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: revert adding PEP 621 metadata, it confuses setuptools #22687

Merged
merged 3 commits into from Nov 30, 2022

Conversation

rgommers
Copy link
Member

See https://github.com/numpy/numpy/pull/22663/files#r1034053192.

The one setup.py tweak is to even make it possible to test newer setuptools versions without build isolation (applies fix suggested in gh-21288).

@rgommers rgommers added the 36 - Build Build related PR label Nov 29, 2022
@rgommers
Copy link
Member Author

Argh, now devpy is unhappy: No project section found in pyproject.toml. @stefanv can that check be removed? There should be no need to use the [project] table, and it's unhelpful to check because it requires adopting PEP 621 support, which then causes probems with setuptools. So during a transition from setuptools to meson, this will be a pain point.

@stefanv
Copy link
Contributor

stefanv commented Nov 29, 2022

Argh, now devpy is unhappy: No project section found in pyproject.toml. @stefanv can that check be removed?

Done

Copy link

@eli-schwartz eli-schwartz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Random bash scripting observation.

tools/wheels/upload_wheels.sh Outdated Show resolved Hide resolved
tools/wheels/upload_wheels.sh Outdated Show resolved Hide resolved
@stefanv
Copy link
Contributor

stefanv commented Nov 30, 2022

Current test failure is:

 ImportError while loading conftest '/home/runner/work/numpy/numpy/build-install/usr/lib/python3.11/site-packages/numpy/conftest.py'.
numpy/__init__.py:109: in <module>
    from ._globals import (
numpy/_globals.py:20: in <module>
    from ._utils import set_module as _set_module
E   ModuleNotFoundError: No module named 'numpy._utils'

I'm confused; I don't see any mention of _utils in _globals.py.

@seberg
Copy link
Member

seberg commented Nov 30, 2022

@stefanv we just added _utils yesterday, sounds like a git clean issue where you got a leftover _utils from the main branch flying around.

@seberg
Copy link
Member

seberg commented Nov 30, 2022

Ahh, I guess weird PR timing there, the meson config just doesn't know about _utils yet.

@seberg
Copy link
Member

seberg commented Nov 30, 2022

OK, I just added _utils to the meson conf, since it makes CI fail on everything now otherwise.

@mattip
Copy link
Member

mattip commented Nov 30, 2022

Can you remove the upload_wheels changes? That will be part of #22690.

@stefanv
Copy link
Contributor

stefanv commented Nov 30, 2022

Can you remove the upload_wheels changes? That will be part of #22690.

Done.

@stefanv
Copy link
Contributor

stefanv commented Nov 30, 2022

This is probably ready to merge then?

@mattip
Copy link
Member

mattip commented Nov 30, 2022

I must be missing something. Where is the information that is commented out going to live? Is it duplicated elsewhere?

Edit: yes, it is in the setup.py

@mattip
Copy link
Member

mattip commented Nov 30, 2022

LGTM. Thanks @rgommers and @stefanv

@mattip mattip merged commit 6812517 into numpy:main Nov 30, 2022
@rgommers rgommers deleted the fix-newer-setuptools branch November 30, 2022 21:06
@mroeschke
Copy link

So with this change it appears that strictly setuptools==59.2.0 is required now to install the nightly numpy wheel?

We have some jobs in pandas to test with the nightly wheels and are encountering this error

https://github.com/pandas-dev/pandas/actions/runs/3589194341/jobs/6041379775

Successfully installed pip-22.3.1 setuptools-65.6.3 wheel-0.38.4
Looking in indexes: https://pypi.anaconda.org/scipy-wheels-nightly/simple
Collecting numpy
  Downloading https://pypi.anaconda.org/scipy-wheels-nightly/simple/numpy/1.25.0.dev0%2B79.g01d64079b/numpy-1.25.0.dev0%2B79.g01d64079b.tar.gz (10.9 MB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 10.9/10.9 MB 13.6 MB/s eta 0:00:00
  Installing build dependencies: started
  Installing build dependencies: finished with status 'error'
  error: subprocess-exited-with-error
  
  × pip subprocess to install build dependencies did not run successfully.
  │ exit code: 1
  ╰─> [3 lines of output]
      Looking in indexes: https://pypi.anaconda.org/scipy-wheels-nightly/simple
      ERROR: Could not find a version that satisfies the requirement setuptools==59.2.0 (from versions: none)
      ERROR: No matching distribution found for setuptools==59.2.0
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: subprocess-exited-with-error

@rgommers
Copy link
Member Author

rgommers commented Dec 1, 2022

That action indeed includes the changes from this PR. It's unlikely that this caused the problem though - this reverted an earlier changed, and all that is left in pyproject.toml is a bunch of commented-out code.

==59.2.0 was always required for an isolated build, and never for a non-isolated build. That should not have changed.

The real question here is why you're getting a build from source, rather than picking up a wheel.

@rgommers
Copy link
Member Author

rgommers commented Dec 1, 2022

The problem is that some macOS wheel builds failed, but an sdist was uploaded to the nightly bucket. @charris @mattip I don't think we should upload an sdist to nightlies at all, that is going to result in these kinds of problems as Pandas is seeing. If a nightly wheel is missing for some platform, one should get a wheel built a week before.

@rgommers
Copy link
Member Author

rgommers commented Dec 1, 2022

@mroeschke I've deleted the most recent sdist, so I think your next CI run should not fail. Could you confirm that after it has run?

@mroeschke
Copy link

Ah thanks @rgommers. A recent build has started to pass now! https://github.com/pandas-dev/pandas/actions/runs/3596938344/jobs/6058167508

@@ -463,6 +463,9 @@ def setup_package():
else:
#from numpy.distutils.core import setup
from setuptools import setup
# workaround for broken --no-build-isolation with newer setuptools,
# see gh-21288
metadata["packages"] = []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need a backport to 1.24.x?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I missed this question. Yes, I think it's a good idea to backport this change. I'll open a PR now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(bit late, sorry about this. it can go into the next bug fix release)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

7 participants