feat: add copy_from method for field assignment #215
Conversation
google-cla
bot
added
the
cla: yes
Codecov Report
@@ Coverage Diff @@
## master #215 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 20 20
Lines 862 895 +33
Branches 149 163 +14
=========================================
+ Hits 862 895 +33
Continue to review full report at Codecov.
|
Because of implementation detatils of the underlying protocol buffers runtime, assigning to a proto-plus message field is achieved by copying, not by updating references. This can lead to surprising and unintuitive behavior for developers who are expecting python object behavior, e.g. reference aliases. This PR adds a 'Message.copy_from' method that is semantically identical to regular assignment. This can be used at the discretion of the developer to clarify expected behavior.
docs/messages.rst
Outdated
|
|
||
| :class:`proto.Message` defines a helper message, :meth:`~.Message.copy_from` to | ||
| help make the distinction clear when reading code. | ||
| The semantics of :meth:`~.Message.copy_from` are addentical to regular assignment. |
There was a problem hiding this comment.
addentical should be identical
And perhaps ... identical to the field assignment behavior described above. would help clarify, as "regular assignment" could imply the behavior most would expect outside the context of protobufs.
There was a problem hiding this comment.
Doh! Good catch. Yes, the proposed wording is more clear.
proto/message.py
Outdated
| """ | ||
| if isinstance(other, type(self)): | ||
| # Just want the underlying proto. | ||
| other = Message.pb(other) |
There was a problem hiding this comment.
This is equivalent to doing other = other._pb right?
There was a problem hiding this comment.
Yes, it is equivalent. The code (and documentation) tries to use the accessor instead of touching the attribute directly.
proto/message.py
Outdated
| if isinstance(other, type(self)): | ||
| # Just want the underlying proto. | ||
| other = Message.pb(other) | ||
| elif isinstance(other, type(Message.pb(self))): |
There was a problem hiding this comment.
Because there's no transformation that needs to occur. It's just here to catch the input type and make sure that it doesn't generate the type mismatch exception below.
proto/message.py
Outdated
| elif isinstance(other, type(Message.pb(self))): | ||
| # Don't need to do anything. | ||
| pass | ||
| elif isinstance(other, collections.abc.Mapping): |
There was a problem hiding this comment.
Is this is in case of a repeated field?
There was a problem hiding this comment.
It's to catch the dictionary syntax:
Message.copy_from(message.submessage, {"name": "Lawrence", "armor_class": 15})|
|
||
| m = Mollusc() | ||
| s = Mollusc.Squid(mass_kg=20) | ||
| Mollusc.Squid.copy_from(m.squid, s) |
There was a problem hiding this comment.
The reason we can't have s.squid.copy_from is because of potential collisions with the proto definition, right?
There was a problem hiding this comment.
Correct. The general problem is that we have to be very, very careful about adding fields and methods to the message class types because of potential name collisions with fields, and the general solution is to add the method (or field) to the metaclass. This means that method invocation becomes a little more indirect: we invoke it from the class and pass in the instance as the first parameter.
🤖 I have created a release \*beep\* \*boop\* --- ## [1.18.0](https://www.github.com/googleapis/proto-plus-python/compare/v1.17.0...v1.18.0) (2021-03-16) ### Features * add copy_from method for field assignment ([#215](https://www.github.com/googleapis/proto-plus-python/issues/215)) ([11c3e58](https://www.github.com/googleapis/proto-plus-python/commit/11c3e58a9ba59f0d7d808a26597dab735ca982ba)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Because of implementation detatils of the underlying protocol buffers
runtime, assigning to a proto-plus message field is achieved by
copying, not by updating references. This can lead to surprising and
unintuitive behavior for developers who are expecting python object
behavior, e.g. reference aliases.
This PR adds a 'Message.copy_from' method that is semantically
identical to regular assignment. This can be used at the discretion of
the developer to clarify expected behavior.