Fix to_(py)?dict bool fields#494
Conversation
|
Hi, thank you for the PR, would you be able to run |
|
I'm very hesitant to make a change of behaviour like this just for bool fields. Does the proposed change apply to all fields which are set and include them even if bool(value) returns False? |
|
yes its applied to all of them and include bool field even if the value is from dataclasses import dataclass
import betterproto
@dataclass
class Greeting(betterproto.Message):
"""Greeting represents a message you can tell a user."""
int_field: int = betterproto.int32_field(1)
bool_field: bool = betterproto.bool_field(2)
if __name__ == "__main__":
greeting = Greeting(
bool_field=False,
)
print("default value is disabled (bool_field=False): ", greeting.to_pydict(include_default_values=False))
print("default value is enabled (bool_field=False): ", greeting.to_pydict(include_default_values=True))
greeting = Greeting()
print("default value is disabled (unset bool_field): ", greeting.to_pydict(include_default_values=False))
print("default value is enabled (unset bool_field): ", greeting.to_pydict(include_default_values=True))output: |
|
Sorry if I wasn't clear, I meant what's the output for things that implement bool that aren't int subclasses. ie what does from dataclasses import dataclass
import betterproto
@dataclass
class Msg(bettterproto.Message):
bar: bool = betterproto.bool_field(1)
@dataclass
class Greeting(betterproto.Message):
"""Greeting represents a message you can tell a user."""
msg_field: Msg = betterproto.message_field(1)
str_field: str = betterproto.string_field(2)
if __name__ == "__main__":
greeting = Greeting(
Msg(), ""
)
print(greeting.to_pydict())print? |
only |
|
Interesting, I would expect an empty dict for the msg field cause it's set |
why? |
|
I'd expect the output to be the output you've shown if the |
|
Please could you add some unit tests for this feature please |
I agree, if a message is placed into a field, I would consider it set and expect there to be an empty dict there, if the message itself has no fields set. |
Fixes #490