Skip to content

Add type stub generation for bazel rules#31791

Closed
kimmywork wants to merge 2 commits intogrpc:masterfrom
kimmywork:bazel_pyi_gen
Closed

Add type stub generation for bazel rules#31791
kimmywork wants to merge 2 commits intogrpc:masterfrom
kimmywork:bazel_pyi_gen

Conversation

@kimmywork
Copy link

Added generating pyi stub option (--pyi_out=...) in py_proto_library.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Dec 2, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

@kimmywork
Copy link
Author

guess labels should be lang/Python and infra/Bazel.
but seems I have no permission to add/edit labels currently.

@drfloob drfloob assigned gnossen and unassigned drfloob Dec 6, 2022
@gnossen
Copy link
Contributor

gnossen commented Feb 8, 2023

@kimmywork Thanks for the contribution! This seems like a good change. Can you please add a test to this directory to make sure that there isn't a regression in this functionality?

I'd imagine the test would do the following:

  • depend on a py_proto_library
  • do some sort of type checking such that the test would fail if you reverted your implementation changes

@grpc-bot
Copy link
Collaborator

More than 30 days have passed since label "disposition/requires reporter action" was added. Closing this issue. Please feel free to re-open/create a new issue if this is still relevant.

@grpc-bot grpc-bot closed this Mar 11, 2023
gnossen pushed a commit that referenced this pull request Jun 12, 2023
With some delay, this is a PR for
#32564 (and previously
#31791).

I looked into adding a regular `py_test` for this change [as
suggested](#31791 (comment))
but I am not aware of any effect that the presence of a .pyi stub file
would have at runtime and where some sort of type-checking in a .py
script would be affected. Stub files are only for use by type checkers &
IDE's. I mean, something like this would work:

```
import helloworld_pb2
py_file = helloworld_pb2.__file__ 
pyi_file = py_file + 'i’
self.assertTrue(os.path.exists(pyi_file))
```

But that seems really hacky to me. Instead I created a simple rule test
for `py_proto_library` with Bazel Skylib which tests the declared
outputs for an example `py_proto_library` target. Indirectly, this also
tests that the declared output files are actually generated. Please let
me know if this is sufficient.
mario-vimal pushed a commit to mario-vimal/grpc that referenced this pull request Jun 15, 2023
With some delay, this is a PR for
grpc#32564 (and previously
grpc#31791).

I looked into adding a regular `py_test` for this change [as
suggested](grpc#31791 (comment))
but I am not aware of any effect that the presence of a .pyi stub file
would have at runtime and where some sort of type-checking in a .py
script would be affected. Stub files are only for use by type checkers &
IDE's. I mean, something like this would work:

```
import helloworld_pb2
py_file = helloworld_pb2.__file__ 
pyi_file = py_file + 'i’
self.assertTrue(os.path.exists(pyi_file))
```

But that seems really hacky to me. Instead I created a simple rule test
for `py_proto_library` with Bazel Skylib which tests the declared
outputs for an example `py_proto_library` target. Indirectly, this also
tests that the declared output files are actually generated. Please let
me know if this is sufficient.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants