-
Notifications
You must be signed in to change notification settings - Fork 763
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
examples: maturin and setuptools_rust examples #1537
Conversation
555c7ab
to
23ba683
Compare
2d80135
to
4f19134
Compare
cc @konstin I've finally almost completed a working set of maturin examples for PyO3! Unfortunately it appears that |
4f19134
to
ac7153d
Compare
Many thanks @konstin - CI is all green here using |
Thanks! |
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.
Two small things, looks really nice otherwise
89b6de0
to
370652e
Compare
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.
Thanks!
for each in 6..version { | ||
println!("cargo:rustc-cfg=Py_3_{}", each); | ||
} | ||
|
||
if implementation == "PyPy" { | ||
println!("cargo:rustc-cfg=PyPy"); | ||
} |
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.
Why do we need this?
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.
This used to be in setup.py, but we now build the tests with maturin instead of setuptools-rust.
We need it because some tests only compile on certain python / PyPy combinations.
IMO this build.rs
method is better anyway because it would still work if we changed the examples back to setuptools_rust
.
It's a little ugly to have to call a Python interpreter here. I have an idea how to split some of pyo3's build.rs
logic into a separate crate pyo3-build-config,
which this build.rs could then re-use. I've been torn for a while on whether that refactoring is worth the additional complexity; maybe I should hurry up and push that PR for a while so that others can see what they think.
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.
I'm going to move this into a new issue so that it doesn't block this PR with all the new examples, which I think has value even if there's this this wart.
Co-authored-by: Yuji Kanagawa <[email protected]>
This builds upon my experience writing the maturin and setuptools_rust examples in #1269
This time I rearrange the example modules as follows:
The two existing packages got a bit of a clean-up. The two new examples
maturin-starter
andsetuptools-rust-starter
are copies of each other tailored to the two different build systems.I made good use of the trick in #1517 (comment) to make importing the submodules easy, and I wanted to show this off in all the examples. (e.g.
from maturin_starter.submodule import SubmoduleClass
).Closes #807