Skip to content

Enum doesn't get mapped on receive #157

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

Closed
upcFrost opened this issue Oct 15, 2020 · 3 comments · Fixed by #293
Closed

Enum doesn't get mapped on receive #157

upcFrost opened this issue Oct 15, 2020 · 3 comments · Fixed by #293
Labels
bug? Bug or feature? help wanted Extra attention is needed

Comments

@upcFrost
Copy link

Hi,

Enums seem not to be mapped on init and receive and left as their integer values.

Example proto:

message TestRequest {
  Category category = 1;
}

enum Category {
  SOMETHING = 0;
  SOMETHING_ELSE = 1;
}

For TestRequest(), the Category value will be int 0 and not SOMETHING.
When receiving category = 1 over gRPC, it will be also left as int 1.

For deserialization it can be fixed easily by adding yet another branch to _postprocess_single. For plain init - I've tried tracing it down, but it seem to go somewhere deep into how dataclasses work

@nat-n nat-n added the bug Something isn't working label Oct 17, 2020
@nat-n
Copy link
Collaborator

nat-n commented Oct 17, 2020

Thanks for creating this issue.

Yes I think it would be nicer if the enum values were used when deserialising or as the default value.

There's an open question of whether values should be altered on assignment, since currently no such magical behaviour exists in betterproto. I suspect it's better not to, though could be convinced otherwise.

Feel free to submit a PR with a solution, or just a failing test case.

@nat-n nat-n added bug? Bug or feature? help wanted Extra attention is needed and removed bug Something isn't working labels Oct 17, 2020
@upcFrost
Copy link
Author

The simplest test case (diff from bf9412e)

diff --git a/tests/test_features.py b/tests/test_features.py
index f548264..f4940d3 100644
--- a/tests/test_features.py
+++ b/tests/test_features.py
@@ -84,6 +84,16 @@ def test_enum_as_int_json():
     foo.bar = 1
     assert foo.to_dict() == {"bar": "ONE"}
 
+def test_enum_mapped_on_init():
+    class TestEnum(betterproto.Enum):
+        ZERO = 0
+        ONE = 1
+
+    @dataclass
+    class Foo(betterproto.Message):
+        bar: TestEnum = betterproto.enum_field(1)
+
+    assert Foo().bar.name == TestEnum.ZERO.name
 
 def test_unknown_fields():
     @dataclass

I would say having enum mapped at least on init and deserialization would be a good idea, as enum has some additional fields (like name in this test), and it might be not very obvious when you can use them and when you can't. In my case, I ran into this issue on request logging (tried to log the enum name).
On __setattribute__ it's probably ok to leave it without any magic mapping, as python is dynamically typed anyway, and there is no real protection against assigning value of some different type to the dataclass field.

@alexifm
Copy link

alexifm commented Jan 25, 2021

from_dict is able to parse an enum

class TestEnum(betterproto.Enum):
    ZERO = 0
    ONE = 1

@dataclass
class Foo(betterproto.Message):
    bar: TestEnum = betterproto.enum_field(1)


f = Foo()
print(f.from_dict(f.to_dict(include_default_values=True)))

g = Foo(1)
print(g.from_dict(g.to_dict()))

Foo(bar=<TestEnum.ZERO: 0>)
Foo(bar=<TestEnum.ONE: 1>)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug? Bug or feature? help wanted Extra attention is needed
Projects
None yet
3 participants