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

Add pyproject.toml #15

Merged
merged 13 commits into from
Nov 22, 2021
Merged

Add pyproject.toml #15

merged 13 commits into from
Nov 22, 2021

Conversation

MuellerSeb
Copy link
Member

@MuellerSeb MuellerSeb commented Nov 16, 2021

This PR enables installing the package with pip locally and should simplify the distribution to pypi and conda in the future.

@MuellerSeb MuellerSeb added the enhancement New feature or request label Nov 16, 2021
@MuellerSeb MuellerSeb added this to the v0.3 milestone Nov 16, 2021
@MuellerSeb MuellerSeb requested a review from LSchueler November 16, 2021 13:28
@MuellerSeb MuellerSeb self-assigned this Nov 16, 2021
@MuellerSeb
Copy link
Member Author

MuellerSeb commented Nov 16, 2021

maybe we should build wheels with cibuildwheel like we did with GSTools.

@adamreichold
Copy link
Contributor

If I understand this correctly, this would limit distribution via PyPI to source distribution requiring Maturin and Rust on the target system? Why not build a single wheel that uses Python's stable API and would thereby work on CPython 3.6 and newer?

@MuellerSeb
Copy link
Member Author

No. Wheels are still build and deployed. But the missing source distribution is now added.

@adamreichold
Copy link
Contributor

Wheels are still build and deployed.

Ah, but only for CPython 3.9, right? Since this is not calling back into Python at all, I see little reason to restrict support from 3.6+?

@MuellerSeb
Copy link
Member Author

When installing from source-distribution, only rust needs to be provided, since maturin is set as build backend and will be installed during the build process. This is the magic of the new pyproject.toml workflow. 😉

@MuellerSeb
Copy link
Member Author

MuellerSeb commented Nov 16, 2021

Wheels are still build and deployed.

Ah, but only for CPython 3.9, right? Since this is not calling back into Python at all, I see little reason to restrict support from 3.6+?

Wheels are build for py3.6 - py3.10 and pypy. This is done automatically by the maturin action. The pinned python version is only used to handle maturin. Before, there were all wheels build multiple times, since the pinned python version is not affecting the built wheels.

@MuellerSeb
Copy link
Member Author

You can see all produced wheels in the provided artifacts when you go to the Github-Actions tab.

@adamreichold
Copy link
Contributor

You can see all produced wheels in the provided artifacts when you go to the Github-Actions tab.

I think this is limited to organisation members.

@MuellerSeb
Copy link
Member Author

For some reason macOS wheels are only build for py3.9

@MuellerSeb
Copy link
Member Author

Guess we have to do it by hand for macOS, like in this workflow: https://github.com/milesgranger/pyrus-cramjam/blob/master/.github/workflows/CI.yml

@adamreichold
Copy link
Contributor

Guess we have to do it by hand for macOS, like in this workflow:

Personally, I would still advise to enable PyO3's abi3 and abi-py36 to build a single binary wheel that works on CPython 3.6 and later (not sure how relevant PyPy support is). It will almost surely cost nothing as this crate does not call back into Python to do its job. And it will reduce the burden of deployment (and build time) in general by having fewer binary wheels.

@adamreichold
Copy link
Contributor

For some reason macOS wheels are only build for py3.9

I suspect the reason is lack of Docker support on GitHub's macOS VM?

@MuellerSeb
Copy link
Member Author

Personally, I would still advise to enable PyO3's abi3 and abi-py36 to build a single binary wheel that works on CPython 3.6 and later (not sure how relevant PyPy support is). It will almost surely cost nothing as this crate does not call back into Python to do its job. And it will reduce the burden of deployment (and build time) in general by having fewer binary wheels.

Just learned about that. Amazing. Will try to do so.

@MuellerSeb
Copy link
Member Author

Worked like a charm. And we only have 7 artifacts.. nice!

@MuellerSeb MuellerSeb linked an issue Nov 18, 2021 that may be closed by this pull request
@MuellerSeb
Copy link
Member Author

@adamreichold, @LSchueler should we include a fix for #16 here?

@adamreichold
Copy link
Contributor

@adamreichold, @LSchueler should we include a fix for #16 here?

I would prefer a separate MR. I think this one is ready for merging when @LSchueler had time to review it while tackling #16 could open up completely new avenues of discussion.

Copy link
Member

@LSchueler LSchueler left a comment

Choose a reason for hiding this comment

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

Thanks you two for putting this together! I was just super busy this week...

@adamreichold already told me about the abi3 support, but unfortunately I didn't have time to join your discussion.

@LSchueler LSchueler merged commit a36f9c6 into main Nov 22, 2021
@LSchueler LSchueler deleted the add_pyproject branch November 22, 2021 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wheels for py3.10 and 32bit systems for py3.6-py3.9
3 participants