[typing] add caffe2/torch proto stubs #52341
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: Add type stubs for caffe2 protos and scripts for generating them. It's worth calling special attention to the following. In order to make `DeviceType`s like `CPU`, `CUDA`, etc. directly accessible from the `caffe2_pb2` module, they are currently freedom-patched into it in `caffe2/python/__init__.py`. This is not ideal: it would be better if these were autogenerated when the protobuf definitions were created by using `allow_alias = true` in the `DeviceTypeProto` definition in `caffe2.proto`. However, it is impossible to do this currently without significant effort. The issue is that the generated proto constants would conflict with various constants defined in the C++ caffe2 codebase in `caffe2_pb.h`. We cannot simply remove these constants and replace them with the caffe2 DeviceTypeProto constants, because a huge portion of code expects `at::DeviceType` constants defined in `core/DeviceType.h` (apparently duplicated to avoid having to figure out how to autogenerate the protobuf definitions using cmake for ATen). Instead, we make a best-effort to add additional definitions in `caffe2_pb2.py` by looking for any freedom-patched constants in `caffe2/python/__init__.py` and making sure they have corresponding stubs in the pyi (see `gen_proto_typestubs_helper.py`). Test Plan: Make sure CI is green; we're just adding stubs. Reviewed By: d4l3k Differential Revision: D26331875 fbshipit-source-id: 4b439e91e93bcb9b10623a1120bef2e918910414
💊 CI failures summary and remediationsAs of commit ecf197e (more details on the Dr. CI page):
🕵️ 5 new failures recognized by patternsThe following CI failures do not appear to be due to upstream breakages:
|
|
This pull request was exported from Phabricator. Differential Revision: D26331875 |
smacke
changed the title
|
This pull request has been merged in a8885ee. |
smacke
added a commit
to smacke/pytorch
that referenced
this pull request
Feb 17, 2021
Summary: After adding .pyi stubs for torch / caffe2 protos, there were some mypy false positives (pytorch#52341). We tell mypy to ignore the offending file here. Test Plan: Let CI run. Reviewed By: dzhulgakov Differential Revision: D26490302 fbshipit-source-id: e1eb47d257ddcb613253eea9a305ca08b8935c9e
facebook-github-bot
pushed a commit
that referenced
this pull request
Feb 17, 2021
Summary: Pull Request resolved: #52370 After adding .pyi stubs for torch / caffe2 protos, there were some mypy false positives (#52341). We tell mypy to ignore the offending file here. Test Plan: Let CI run. Reviewed By: malfet, dzhulgakov Differential Revision: D26490302 fbshipit-source-id: 87cdfd7419efdc7abece9ca975a464201732b7a0
xsacha
pushed a commit
to xsacha/pytorch
that referenced
this pull request
Mar 31, 2021
Summary: Pull Request resolved: pytorch#52341 Add type stubs for caffe2 protos and scripts for generating them. It's worth calling special attention to the following. In order to make `DeviceType`s like `CPU`, `CUDA`, etc. directly accessible from the `caffe2_pb2` module, they are currently freedom-patched into it in `caffe2/python/__init__.py`. This is not ideal: it would be better if these were autogenerated when the protobuf definitions were created by using `allow_alias = true` in the `DeviceTypeProto` definition in `caffe2.proto`. However, it is impossible to do this currently without significant effort. The issue is that the generated proto constants would conflict with various constants defined in the C++ caffe2 codebase in `caffe2_pb.h`. We cannot simply remove these constants and replace them with the caffe2 DeviceTypeProto constants, because a huge portion of code expects `at::DeviceType` constants defined in `core/DeviceType.h` (apparently duplicated to avoid having to figure out how to autogenerate the protobuf definitions using cmake for ATen). Instead, we make a best-effort to add additional definitions in `caffe2_pb2.py` by looking for any freedom-patched constants in `caffe2/python/__init__.py` and making sure they have corresponding stubs in the pyi (see `gen_proto_typestubs_helper.py`). Test Plan: Make sure CI is green; we're just adding stubs. Reviewed By: d4l3k Differential Revision: D26331875 fbshipit-source-id: 2eea147e5bf393542f558ff8cf6385c47624b770
xsacha
pushed a commit
to xsacha/pytorch
that referenced
this pull request
Mar 31, 2021
Summary: Pull Request resolved: pytorch#52370 After adding .pyi stubs for torch / caffe2 protos, there were some mypy false positives (pytorch#52341). We tell mypy to ignore the offending file here. Test Plan: Let CI run. Reviewed By: malfet, dzhulgakov Differential Revision: D26490302 fbshipit-source-id: 87cdfd7419efdc7abece9ca975a464201732b7a0
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Add type stubs for caffe2 protos and scripts for generating them.
It's worth calling special attention to the following. In order to make
DeviceTypes likeCPU,CUDA, etc. directly accessible from thecaffe2_pb2module, they are currently freedom-patched into it incaffe2/python/__init__.py. This is not ideal: it would be better if these were autogenerated when the protobuf definitions were created by usingallow_alias = truein theDeviceTypeProtodefinition incaffe2.proto.However, it is impossible to do this currently without significant effort. The issue is that the generated proto constants would conflict with various constants defined in the C++ caffe2 codebase in
caffe2_pb.h. We cannot simply remove these constants and replace them with the caffe2 DeviceTypeProto constants, because a huge portion of code expectsat::DeviceTypeconstants defined incore/DeviceType.h(apparently duplicated to avoid having to figure out how to autogenerate the protobuf definitions using cmake for ATen).Instead, we make a best-effort to add additional definitions in
caffe2_pb2.pyby looking for any freedom-patched constants incaffe2/python/__init__.pyand making sure they have corresponding stubs in the pyi (seegen_proto_typestubs_helper.py).Test Plan: Make sure CI is green; we're just adding stubs.
Reviewed By: d4l3k
Differential Revision: D26331875