-
Notifications
You must be signed in to change notification settings - Fork 520
feat: pyo3 support module prefix + naming #3726
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
base: main
Are you sure you want to change the base?
Conversation
7a660ad to
3a98fed
Compare
35bc452 to
e19bd9c
Compare
UebelAndre
left a comment
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! Just one question
| load("//:defs.bzl", "pyo3_extension") | ||
|
|
||
| pyo3_extension( | ||
| name = "module_prefix", |
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 add these attributes and not change the target name to foo/bar?
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 tried that first, e.g. for string_sum:
pyo3_extension(
name = "foo/string_sum",
srcs = ["string_sum.rs"],
edition = "2021",
)
Building that target yields this error:
Error in fail: Crate name 'foo/string_sum' contains invalid character(s): /
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.
There were several quirks with getting the names to line up (pymodule <-> output binary <-> crate name) + getting the outputs in the right directory so Python registers it as nested module rather than a top level module. Adding these (hopefully simple) knobs seemed like the cleanest approach.
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 nervous about the amount of indirection this introduces. What is the use case here that directory structure can't be made to what you want?
Adds support for specifying the module name + module prefix for pyo3 extensions.