Skip to content
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

Fix pydict serialization for optional fields #495

Conversation

nickderobertis
Copy link
Contributor

@nickderobertis nickderobertis commented May 27, 2023

What

Adds a block in Mesaage.to_pydict that explicitly checks for None values and assigns them to the dict if include_default_values=True.

Why

Fixes #475 where to_pydict fails on optional fields with the error:

  File "...", in <listcomp>
    json.dump([f.to_pydict(casing=Casing.SNAKE, include_default_values=True) for f in frames], file)
  File "...\Python\Python310\lib\site-packages\betterproto\__init__.py", line 1365, in to_pydict
    output[cased_name] = value.to_pydict(casing, include_default_values)
  File "...\Python\Python310\lib\site-packages\betterproto\__init__.py", line 1359, in to_pydict
    value._serialized_on_wire
AttributeError: 'NoneType' object has no attribute '_serialized_on_wire'

Testing

Added a test case that checks to_pydict and to_dict output when passing the message value and not, and when passing include_default_values and not.

Copy link
Collaborator

@Gobot1234 Gobot1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR

@Gobot1234 Gobot1234 merged commit fcbd8a3 into danielgtaylor:master May 28, 2023
@nickderobertis nickderobertis deleted the nickderobertis/475/fix-dict-serialization-optional branch May 29, 2023 17:32
@nickderobertis
Copy link
Contributor Author

Thanks for reviewing and merging it quickly! When do you think we'll cut a new release with this change? Trying to see if I need to peg to git version or if it will be available in a v2 beta release soon.

@Gobot1234
Copy link
Collaborator

I can't make any guarantees as I can't make releases by myself so I'll have to recommend that you pin to a git rev, sorry.

@nickderobertis
Copy link
Contributor Author

Ok that makes sense, thank you for the response.

bbonenfant pushed a commit to pachyderm/python-betterproto that referenced this pull request Jan 23, 2024
bbonenfant added a commit to pachyderm/python-betterproto that referenced this pull request Jan 23, 2024
# Conflicts:
#	tests/test_features.py

# Conflicts:
#	tests/test_features.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError in to_pydict with optional fields
2 participants