-
Notifications
You must be signed in to change notification settings - Fork 14
Add back struct support #102
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
Conversation
|
Is this related to danielgtaylor/python-betterproto#599? I'd love if Struct were smarter :D. Right now we do this: def struct_from_dict(d: dict[str, Any]):
"""
struct_from_dict converts the given dictionary into a proto Struct.
This is a sadly needed because of https://github.com/danielgtaylor/python-betterproto/issues/599.
"""
s = LegacyStruct()
s.update(d)
return Struct.FromString(s.SerializeToString())
def struct_to_dict(s: Struct) -> dict[str, Any]:
"""struct_to_dict converts a proto Struct into a python dictionary."""
output: dict[str, Any] = {}
for key, value in s.fields.items():
ret = value_to_py(value)
output[key] = ret
return output
def value_to_py(proto_value: Value, include_default_values: bool = False) -> Any | None:
"""value_to_py converts a proto Value to a native python value."""
field_name, actual_value = betterproto2.which_one_of(proto_value, "kind")
match field_name:
case "null_value" | "number_value" | "string_value" | "bool_value":
return actual_value
case "struct_value":
return struct_to_dict(typing.cast(Struct, actual_value))
case "list_value":
return [value_to_py(list_val) for list_val in typing.cast(ListValue, actual_value).values]
case _:
return None |
|
I am currently monkey patching the original betterproto to circumvent the bug I report I opened danielgtaylor/python-betterproto#599 """
Monkey patch betterproto to have Struct behave as expected.
In particular, the from_dict and to_dict methods of betterproto.lib.google.protobuf.Struct
do not work as expected, as they expect the values to be messages of type Value, while
protobuf Struct behaves as if the values are inlined in the struct.
After calling monkeypatch_betterproto_struct(), you can use from_dict and to_dict as expected.
Example:
>>> monkeypatch_betterproto_struct() # this is automatically done if you import amp.kit (or any module that imports it)
>>> from betterproto.lib.google.protobuf import Struct, Value
>>> import json
>>> s = Struct.from_dict({"a": 42, "b": "hello"})
>>> s.fields["b"]
Value(string_value='hello')
>>> json.loads(s.to_json()) == {"a": 42, "b": "hello"}
True
"""
from typing import Any, Mapping
import betterproto.lib.google.protobuf
from betterproto import Casing
from betterproto.lib.google.protobuf import Struct
from google.protobuf.json_format import MessageToDict as pb_MessageToDict
from google.protobuf.struct_pb2 import Struct as pb_Struct
def monkeypatch_betterproto_struct():
"""Monkey patch betterproto to have Struct behave"""
def struct_to_dict_method(
self: Struct,
casing: Casing = Casing.CAMEL, # type: ignore The Casing.CAMEL is not a real enum value...
include_default_values: bool = False,
) -> dict[str, Any]:
# Protobuf Struct special case things, in particular it behaves as if values are inlined
# in the struct, while betterproto sticks to the .proto definition of Struct,
# that is values in the dict have to be messages of type Value.
# So to_dict and to_json (which is just json_dumps(to_dict(()) do not work as expected.
#
# Fix: we create the corresponding proto struct, then use proto tool to get the dict representation with inlined values.
s = pb_Struct()
s.ParseFromString(self.SerializeToString())
return pb_MessageToDict(s, preserving_proto_field_name=True)
# Monkey patch betterproto Struct to_dict and to_pydict (they are the same for Struct, no custom types expected)
betterproto.lib.google.protobuf.Struct.to_dict = struct_to_dict_method
betterproto.lib.google.protobuf.Struct.to_pydict = struct_to_dict_method
def struct_from_dict_method(self: Struct, value: Mapping[str, Any]) -> Struct:
# Same reasoning as to_dict above.
# Issue: from_dict and from_json (which is just from_dict(json.loads()) do not work as expected (values not inlined).
# Fix: we create the struct using protobuf, serialize it and then parse it with betterproto.
s = pb_Struct()
s.update(value)
return self.FromString(s.SerializeToString())
# Betterproto uses hybridmethod for from_dict, i.e. it works as class method and instance method.
# so we have to create the hybridmethod correspondingly.
from betterproto.utils import hybridmethod
def struct_from_dict_classmethod(
cls: type[Struct], value: Mapping[str, Any]
) -> Struct:
return struct_from_dict_method(cls(), value)
from_dict = hybridmethod(struct_from_dict_classmethod)
from_dict.instance_func = struct_from_dict_method
betterproto.lib.google.protobuf.Struct.from_dict = from_dict |
|
My monkey patch is "working" but does not abide by a bunch of things like casing, etc. |
4b21bfb to
56c3ce5
Compare
56c3ce5 to
a901744
Compare
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.
Pull Request Overview
This PR adds back struct support by converting the pack method of the Any class from an instance method to a classmethod, allowing it to return a new Any instance instead of modifying an existing one. It also implements comprehensive support for Google Protocol Buffer struct types including Struct, ListValue, and Value with proper serialization/deserialization capabilities.
- Makes
Any.pack()a classmethod that returns a newAnyinstance - Adds
from_dictandto_dictmethods forAny,Struct,ListValue, andValueclasses - Updates tests to use the new classmethod API and removes expected failures for struct-related tests
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| betterproto2_compiler/src/betterproto2_compiler/lib/google/protobuf/init.py | Converts Any.pack() to classmethod and adds struct support methods directly to generated protobuf classes |
| betterproto2_compiler/src/betterproto2_compiler/known_types/struct.py | Implements enhanced struct classes with proper dict conversion methods |
| betterproto2_compiler/src/betterproto2_compiler/known_types/any.py | Converts Any.pack() to classmethod and adds from_dict method |
| betterproto2_compiler/src/betterproto2_compiler/known_types/init.py | Registers new struct methods in the known types system |
| betterproto2_compiler/pyproject.toml | Updates dependency configuration to use local betterproto2 package |
| betterproto2/tests/test_struct.py | Adds comprehensive tests for struct functionality |
| betterproto2/tests/test_pickling.py | Updates tests to use new Any.pack() classmethod API |
| betterproto2/tests/test_inputs.py | Removes expected failures for struct-related tests |
| betterproto2/tests/test_any.py | Updates tests to use new Any.pack() classmethod API |
| betterproto2/tests/inputs/config.py | Removes test configuration file with expected failures |
| betterproto2/src/betterproto2/init.py | Adds type alias and improves serialization logic for repeated and map fields |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| return output | ||
|
|
||
| if type(value).to_dict == betterproto2.Message.to_dict: | ||
| if type(value).to_dict is betterproto2.Message.to_dict: |
Copilot
AI
Aug 22, 2025
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.
This identity comparison may fail if classes are reloaded or defined differently. The original == comparison was safer for checking if the method is overridden.
| if type(value).to_dict is betterproto2.Message.to_dict: | |
| if type(value).to_dict == betterproto2.Message.to_dict: |
| return output | ||
|
|
||
| if type(value).to_dict == betterproto2.Message.to_dict: | ||
| if type(value).to_dict is betterproto2.Message.to_dict: |
Copilot
AI
Aug 22, 2025
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.
This identity comparison may fail if classes are reloaded or defined differently. The original == comparison was safer for checking if the method is overridden.
| if type(value).to_dict is betterproto2.Message.to_dict: | |
| if type(value).to_dict == betterproto2.Message.to_dict: |
| if not msg_cls: | ||
| raise TypeError(f"Can't unpack unregistered type: {type_url}") | ||
|
|
||
| if msg_cls.to_dict is not betterproto2.Message.to_dict: |
Copilot
AI
Aug 22, 2025
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.
This identity comparison may fail if classes are reloaded or defined differently. Consider using a more robust method to check if the method is overridden.
| if msg_cls.to_dict is not betterproto2.Message.to_dict: | |
| if "to_dict" in msg_cls.__dict__: |
| raise TypeError(f"Can't unpack unregistered type: {type_url}") | ||
|
|
||
| if msg_cls.to_dict is not betterproto2.Message.to_dict: | ||
| value = value["value"] |
Copilot
AI
Aug 22, 2025
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.
This identity comparison may fail if classes are reloaded or defined differently. Consider using a more robust method to check if the method is overridden.
| value = value["value"] | |
| # Check if to_dict is overridden in msg_cls (not inherited directly from betterproto2.Message) | |
| for cls_in_mro in inspect.getmro(msg_cls): | |
| if "to_dict" in cls_in_mro.__dict__: | |
| if cls_in_mro is not betterproto2.Message: | |
| value = value["value"] | |
| break |
packmethod ofAnya class method.Struct,ValueandListValue