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

Bump CIBuildWheel Version to 2.15.0 #70

Merged
merged 10 commits into from
Aug 16, 2023
Merged

Bump CIBuildWheel Version to 2.15.0 #70

merged 10 commits into from
Aug 16, 2023

Conversation

Eric-Vin
Copy link

To address Issue #69

@Eric-Vin Eric-Vin mentioned this pull request Aug 15, 2023
@mikedh
Copy link
Collaborator

mikedh commented Aug 15, 2023

Thanks for the PR! Can you bump version.py too for the release process?

@Eric-Vin
Copy link
Author

Done! I went with the most minor version increment but let me know if you'd like something else.

@mikedh
Copy link
Collaborator

mikedh commented Aug 15, 2023

Thanks for the fixes!! Sorry I'd fix these locally but I don't have the project open. I think to fix the build we need to:

  • In pyproject.toml change Cython -> Cython<3 from how to compile python-fcl wheel from source? #67
  • locally run pip install --upgrade black and run the latest version of black on the tests (and/or pin the version of black in the github actions YAML).

@Eric-Vin
Copy link
Author

No problem at all, thank you for your help! I cloned the repo and made the changes you requested, and it looks like the wheels build with pip wheel ./ --no-deps -w wheelhouse/, at least on Linux.

@Eric-Vin
Copy link
Author

Hmm, looks like black is still failing, I'll see about fixing that.

@mikedh
Copy link
Collaborator

mikedh commented Aug 15, 2023

Looks close! It seems like everything except 3.12 is working, which appears to be surfacing a bug/deprecation in setuptools? Looks like it's this one: pypa/setuptools#3935

@Eric-Vin
Copy link
Author

Ah yep, distutils is deprecated in 3.12 according to https://peps.python.org/pep-0632/. Unfortunately I'm not really sure how to fix this one.

@mikedh
Copy link
Collaborator

mikedh commented Aug 15, 2023

Yeah 🥲. I think we're blocked until numpy/numpy#23808 gets fixed?

@mikedh
Copy link
Collaborator

mikedh commented Aug 15, 2023

Oh actually this looks like it could plausibly work:
numpy/numpy#23808 (comment)

[build-system]
requires = [
  'numpy; python_version<"3.12.0rc1"',
  'numpy>=1.26.0b1; python_version>="3.12.0.rc1"',
  # ...
]

@Eric-Vin
Copy link
Author

Eric-Vin commented Aug 15, 2023

Seems to work in 3.12 now! Hopefully the actions pass. I made a slightly different version that kept oldest-supported-numpy for older versions, so as to avoid breaking anything, but I can definitely drop that if you'd like.

@mikedh
Copy link
Collaborator

mikedh commented Aug 15, 2023

Looks like it's building now, only failing on 3.12 tests 😆. Thanks for all the followup on this PR!

It's a little suspicious python-fcl currently only has numpy in the build-system (which I don't know if the install respects?) I suspect it might need the version-locked numpy in project.dependencies too:

[project]
name = "python-fcl"
dependencies = ["numpy ...version-info-here"]

@Eric-Vin
Copy link
Author

Ah good point. With this change I can now install properly in 3.12, so hopefully it'll fix the problem.

@Eric-Vin
Copy link
Author

Shoot now it looks like there's a mismatch....

@mikedh
Copy link
Collaborator

mikedh commented Aug 15, 2023

Oh I wonder if this version range needs something for Python 3.6, or to drop Python 3.6. FWIW I've had to drop 3.6 on other projects due to pyproject.toml support and it would probably be fine to do that here if necessary.

ERROR: Some build dependencies for file:///project conflict with the backend dependencies: numpy==1.13.3 is incompatible with numpy>=1.26.0b1; python_version >= "3.12".

@Eric-Vin
Copy link
Author

Hmm I would think it would just work, since for older versions like 3.6 it should fall back to oldest-supported-numpy. I wonder if it'll work if I change oldest-supported-numpy back to numpy in setup.cfg, since that's what it was originally.

@mikedh
Copy link
Collaborator

mikedh commented Aug 16, 2023

Oh looking at the logs my theory on this is that we're double-specifying the build system in both setup.cfg and pyproject.toml:

  • setup.cfg is used on Python 3.6, and specified in setup_requires
  • pyproject.toml is used on Python 3.7+ and is specified in build

@Eric-Vin
Copy link
Author

Eric-Vin commented Aug 16, 2023

Hmm, so does that mean we would want something like this?

setup_requires =
    numpy
    Cython
install_requires =
    numpy; python_version<'3.12'
    numpy>=1.26.0b1; python_version>='3.12'
    Cython

Since if we take the numpy specification out of both blocks in setup.cfg we'd get back to an earlier version which also had a crash. I'll push the above so we can give it a try.

@mikedh
Copy link
Collaborator

mikedh commented Aug 16, 2023

Looks like you got it great work! Thanks for the follow ups, these CI changes are particularly rough haha. I think at some point this project should probably use ini2toml to move everything from setup.cfg to pyproject.toml and drop 3.6 which would probably make this build a lot more stable.

@mikedh mikedh merged commit c1e2cdb into BerkeleyAutomation:master Aug 16, 2023
4 checks passed
@Eric-Vin
Copy link
Author

Great! Thanks for guiding me through the process and for maintaining the project!

mikedh added a commit to trimesh/embreex that referenced this pull request Aug 24, 2023
mikedh referenced this pull request in trimesh/embreex Sep 15, 2023
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.

2 participants