-
Notifications
You must be signed in to change notification settings - Fork 15.7k
Generate .pyi files in py_proto_library (#10366) #21567
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
We here adjust py_proto_library to produce `.pyi` files along with the `.py` files it already generates. This achieves the same effect as the gRPC py_proto_library. Closes protocolbuffers#10366.
+1; with |
@anandolee Thanks for adding the tag! It looks like my PR passes CI, so I look forward to your review. |
) | ||
|
||
# Generated sources == Python sources | ||
python_sources = generated_sources | ||
python_sources = generated_sources + generated_stubs |
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.
We discussed this with some python folks, and while we agree this PR looks like a good approach, it needs a few tweaks. Specifically, the pyi files should end up in the new direct_pyi_files
/transitive_pyi_files
fields of PyInfo
added in rules_python 1.1, instead of getting globbed together into transitive_sources
with the py files like is currently happening
I think this is just a matter of piping the information through _PyProtoInfo, and should be a relatively minor tweak to your PR though
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.
@mkruskal-google Thank you for your feedback. At present, it looks like protobuf requires rules_python 1.0.0 in MODULE.bazel
and protobuf_deps.bzl
Are you asking for me to bump that to rules_python 1.1.0 as part of this PR or possibly a separate PR? Or should I attempt to be compatible with rules_python 1.0.0 but take advantage of the new PyInfo
fields if they are available?
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.
Those are the two options :). I think bumping to 1.1 should be fine as it's a minor version release, and it would be a lot easier. We could go the extra mile to support 1.0.0 by checking for the presence of the new fields, but I don't think it's necessary here
We here adjust py_proto_library to produce
.pyi
files along with the.py
files it already generates. This achieves the same effect as the gRPC py_proto_library.Closes #10366.