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: to_dict returns wrong enum fields when numbering is not consecutive #102

Merged

Conversation

boukeversteegh
Copy link
Collaborator

closes #93

  • When converting to dictionary, get enum values by Enum() constructor, instead of index.
  • Small refactor: determine if a field is a list, not by its value, but by its type. (i.e. do not rely on the user to tell us what type our fields are).
  • Small refactor: removal of magic constants that were proto3 type names.
  • Added more tests for enum.

@boukeversteegh boukeversteegh added bug Something isn't working small Low effort issue that can easily be picked up labels Jul 4, 2020
@boukeversteegh boukeversteegh requested a review from nat-n July 8, 2020 21:10
Comment on lines 846 to 855
if field_is_repeated:
enum_class = field_type.__args__[0]
if isinstance(value, typing.Iterable):
output[cased_name] = [
enum_class(element).name for element in value
]
else:
warnings.warn(
f"Non-iterable value for repeated enum field {field_name}"
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The actual fix is here ☝️

@boukeversteegh boukeversteegh merged commit 6c29771 into danielgtaylor:master Jul 12, 2020
Gobot1234 pushed a commit to Gobot1234/python-betterproto that referenced this pull request Aug 24, 2020
…ive (danielgtaylor#102)

Fixes danielgtaylor#93 to_dict returns wrong enum fields when numbering is not consecutive
@abn abn mentioned this pull request Nov 24, 2020
@bencwallace
Copy link

Thanks for this bug fix! I just ran into this myself and it took me a while to realize the fix was available in version 2. Any when this will come out of beta?

@Gobot1234
Copy link
Collaborator

Thanks for this bug fix! I just ran into this myself and it took me a while to realize the fix was available in version 2. Any when this will come out of beta?

No but if you want this fix install betterproto==2.0.0b2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working small Low effort issue that can easily be picked up
Projects
None yet
Development

Successfully merging this pull request may close these issues.

to_dict returns wrong enum fields when numbering is not consecutive
4 participants