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

Implement Message.__bool__ #142

Merged
merged 13 commits into from
Nov 24, 2020
Merged

Implement Message.__bool__ #142

merged 13 commits into from
Nov 24, 2020

Conversation

Gobot1234
Copy link
Collaborator

@Gobot1234 Gobot1234 commented Aug 24, 2020

Carries on from #138, I messed up the git tree.

@danielgtaylor
Copy link
Owner

danielgtaylor commented Sep 4, 2020

This look interesting. I like the general idea for sure.

Given proto's use of default zero values I wonder if it's more useful to know:

  1. Has anything ever been set on this?
  2. Is the message equivalent to zero (i.e. all fields are their zero value)?

This PR seems to implement 1 unless I'm mistaken. Thoughts?

To elaborate, you can see that a Python list will return to False after you explicitly "set" it by removing the item:

>>> test = []
>>> bool(test)
False

>>> test.append(1)
>>> bool(test)
True

>>> test.pop()
1
>>> bool(test)
False

I think the proto equivalent is setting the int32 to 0 since that should mean it never gets sent over the wire since it's the zero value for the type.

@Gobot1234
Copy link
Collaborator Author

Am I mistaken in thinking that returning self._serialized_on_wire would achieve this?

@Gobot1234
Copy link
Collaborator Author

I should have this working as a collection would now, so if a value is set to anything and then has its value set to its default it will return False

@nat-n
Copy link
Collaborator

nat-n commented Oct 19, 2020

@Gobot1234 I think we can merge this in principal, but it's significant an API feature that it should really be documented. Would you mind?

@Gobot1234
Copy link
Collaborator Author

Ah yes of course.

@Gobot1234
Copy link
Collaborator Author

Sorry just realised I haven't added it to the Sphinx special members directive yet

@nat-n nat-n merged commit 69dfe9c into danielgtaylor:master Nov 24, 2020
@abn abn mentioned this pull request Nov 24, 2020
w4rum pushed a commit to w4rum/python-betterproto that referenced this pull request Dec 1, 2020
* Implement Message.__bool__ with similar semantics to a collection, such that any value being set on the message (i.e. having a non-default value) make the Message value truthy .

Co-authored-by: nat <n@natn.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants