Repository navigation
Conversation
serialize() left its instance parameter unannotated, so strict type checkers report the method as partially unknown on every call. deserialize() returned the base Message, so callers had to narrow the result back to the class they called it on. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request updates the type annotations in packages/proto-plus/proto/message.py. Specifically, it introduces a generic type variable _MessageT bound to Message and applies it to the deserialize method. It also adds type hints to the serialize method's instance parameter, allowing Message, message.Message, or a Mapping[str, Any]. There are no review comments, and I have no feedback to provide.
MessageMeta.serialize()leavesinstanceunannotated, so pyright in strict mode reportsType of "serialize" is partially unknownat every call site.MessageMeta.deserialize()is annotated-> "Message", soMyMessage.deserialize(b)types as the baseMessageand callers have toisinstance-narrow it back toMyMessagebefore touching its fields.serialize(cls, instance: Union["Message", message.Message, Mapping[str, Any]]): the three shapes the docstring's "accepted by the type's constructor" covers.deserialize(cls: Type[_MessageT], payload: bytes) -> _MessageT: the same metaclass self-type pattern typeshed uses forEnumMeta.MyMessage.deserialize(b)now types asMyMessage.Annotation-only, no runtime change.
pytest testsinpackages/proto-plus: 274 passed, 2 skipped. Checked with pyright strict against a generated google-ads message:GoogleAdsFailure.deserialize(b"")revealsGoogleAdsFailure, andserializeis fully known.