python-betterproto icon indicating copy to clipboard operation
python-betterproto copied to clipboard

Enum doesn't get mapped on receive

Open upcFrost opened this issue 5 years ago • 3 comments

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

upcFrost avatar Oct 15 '20 09:10 upcFrost

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 avatar Oct 17 '20 13:10 nat-n

The simplest test case (diff from bf9412e08379211bee4698f64e05d9a512b59ed6)

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.

upcFrost avatar Oct 17 '20 17:10 upcFrost

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>)

alexifm avatar Jan 25 '21 23:01 alexifm