-
Notifications
You must be signed in to change notification settings - Fork 214
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
TypeError in to_pydict with optional fields #475
Comments
You should use to_pydict for this, using strings is for compatibility purposes with the google implementation |
to_pydict throws the following error:
|
You are probably breaking type safety somewhere which isn't really on the libraries shoulders to deal with. If you have a code snippet that passes type checking and reproduces this please do send it. |
Entry point from data.proto import Frame
Frame().to_pydict()
# Generated by the protocol buffer compiler. DO NOT EDIT!
@dataclass(eq=False, repr=False)
class HeaderFrame(betterproto.Message):
test: bool = betterproto.bool_field(2)
@dataclass(eq=False, repr=False)
class Frame(betterproto.Message):
header: Optional["HeaderFrame"] = betterproto.message_field(
2, optional=True, group="_Header"
) Causes
I'm using betterproto version 2.0.0b5 This is just a quick way to reproduce the error, but it also happens with messages deserialized using |
I just ran into this problem as well, and it seems like it would be easy to fix by just adding a clause to check if the value is elif value is None:
output[cased_name] = value I tried to pull down the repo to make this change an add a test, but I'm having some problems getting the development environment working. Poetry failed to install, so I ran |
What version of python are you trying to build the repo with and what failed to build when you ran |
I was originally trying with 3.10 on Mac OS, now just tried with Python 3.11.3, Poetry 1.4.2 on Ubuntu 22.04:
There is a lot more output before that as well, let me know if it's helpful. |
Try using 3.9 a lot of projects in the toolchain still aren't 3.10+ compatible |
Ok great, thanks, poetry install worked on 3.9. However, I am still getting the same errors when running
|
Did you install all the developer requirements as well? |
Ahh thank you I missed adding the I'll go ahead and first create failing test cases for this and then work on a fix. Should I put up the PR once I have failing test cases or all together? |
I'd probably wait for both before making a pr |
Sounds good, PR is finished with both! Happy to make adjustments for any feedback but it seems pretty straightforward. |
See #475 (comment)
Outdated
I am using betterproto to load a ton of messages at once from a binary file, however this operation is very slow so I wanted to cache/parse it into json using
json.dump
withto_dict
, and then load it directly from json. Problem is thatfrom_json
is also very slow, so my idea was to useto_dict(casing=Casing.SNAKE, include_default_values=true)
to generate a dictionary that matches the attributes of the message dataclass, and then wrap it in an utility class that redirects dict.attribute calls to dict["attribute"] so that for the consumer (which is using the message dataclasses exclusively for their data) there's no difference.However the problem is that when serializing to dictionary, enum values are serialized based on their name instead of value. Maybe an optional parameter can be added to change this behaviour? Thank you
Utility dictionary class for redirecting attributes to items
The text was updated successfully, but these errors were encountered: