-
Notifications
You must be signed in to change notification settings - Fork 220
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_(py)?dict bool fields #494
base: master
Are you sure you want to change the base?
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