FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Implement Message.__bool__ by Gobot1234 · Pull Request #142 · danielgtaylor/python-betterproto · GitHub

Implement Message.__bool__ - #142

Merged
nat-n merged 13 commits into
danielgtaylor:masterfrom
Gobot1234:master
Nov 24, 2020
Merged

Implement Message.__bool__#142
nat-n merged 13 commits into
danielgtaylor:masterfrom
Gobot1234:master

Conversation

Gobot1234 commented Aug 24, 2020
edited
Loading

Copy link
Copy Markdown
Collaborator

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

nat-n requested a review from danielgtaylor September 2, 2020 19:42

danielgtaylor commented Sep 4, 2020
edited
Loading

Copy link
Copy Markdown
Owner

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.

Copy link
Copy Markdown
Collaborator Author

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

Copy link
Copy Markdown
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 commented Oct 19, 2020

Copy link
Copy Markdown
Collaborator

@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?

Copy link
Copy Markdown
Collaborator Author

Ah yes of course.

Copy link
Copy Markdown
Collaborator Author

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

nat-n merged commit 69dfe9c into danielgtaylor:master Nov 24, 2020
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL