-
Notifications
You must be signed in to change notification settings - Fork 218
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
feat: added ParseFromString method to message for compatibility #336
base: master
Are you sure you want to change the base?
feat: added ParseFromString method to message for compatibility #336
Conversation
…official protobuf method
d0b5cd2
to
cb9161b
Compare
Thanks! |
@@ -1022,6 +1022,29 @@ def FromString(cls: Type[T], data: bytes) -> T: | |||
""" | |||
return cls().parse(data) | |||
|
|||
# For compatibility with Google protobuf official implementation. | |||
def ParseFromString(self, data: bytes) -> T: |
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.
def ParseFromString(self, data: bytes) -> T: | |
def ParseFromString(self: T, data: bytes) -> T: |
:class:`Message` | ||
The initialized message. | ||
""" | ||
return self.parse(data) |
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.
Isn't this slightly different from the offical implementation which clears any currently set values? https://github.com/protocolbuffers/protobuf/blob/2a2a9b6e64e0cda49b0e5377cad58c790087e1c9/python/google/protobuf/message.py#L193-L199
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 indeed seems to be the case.
I've added a simple example which shows the difference in result from ParseFromString
with the one from this PR:
syntax = "proto3";
package some.package.name;
message Cat {
int64 age = 1;
int64 lives = 2;
}
def ParseFromString(self, data: bytes):
return self.parse(data)
if __name__ == '__main__':
# Create bytes
cat = Cat(
age=5,
# lives=9,
)
data = cat.SerializeToString()
# Dynamically add ParseFromString to the Cat class
Cat.ParseFromString = ParseFromString
# ParseFromString for the betterproto cat and the pb2 cat
cat_betterproto = Cat(age=3, lives=8)
cat_pb2 = Cat_pb2(age=3, lives=8)
cat_betterproto.ParseFromString(data)
cat_pb2.ParseFromString(data)
print(cat_betterproto) # Cat(age=5, lives=8), lives are not cleared
print(cat_pb2) # age: 5, lives are cleared
assert cat_pb2.lives == 0
This PR adds the ParseFromString attribute to the message class for compatibility with the official Google protobuf implementation and method exposure.
Closes #323