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

Add documentation for __protobuf__ #244

Closed
fischor opened this issue Aug 28, 2021 · 2 comments · Fixed by #409
Closed

Add documentation for __protobuf__ #244

fischor opened this issue Aug 28, 2021 · 2 comments · Fixed by #409
Labels
type: docs Improvement to the documentation for an API.

Comments

@fischor
Copy link

fischor commented Aug 28, 2021

It would be nice to have documentation on how to use the __protobuf__ = proto.module magic and what it actually does.
I've seen it in use here:

https://github.com/googleapis/python-automl/blob/227c08b2fa6efa5af6677efc8e35382b23915fb4/google/cloud/automl_v1/types/classification.py#L19-L26

What is the manifest param for?
It seems that nested messages do not need to be registered in there. Is that correct?

One problem I was facing: if you have two messages with the same name and no __protobuf__ specified, you get an error:

# a.py

class A(proto.Message):
    name = proto.Field(proto.STRING, number=1)
# b.py

class A(proto.Message):
    name = proto.Field(proto.STRING, number=1)
# main.py

import a
import b

_a = a.A(name="Hello, A!")
_b = b.A(name="Hello, B!")
TypeError: Couldn't build proto file into descriptor pool!
Invalid proto descriptor for file "b_a.proto":
  A.name: "A.name" is already defined in file "a_a.proto".
  A: "A" is already defined in file "a_a.proto".

So I assume with __protobuf__ = proto.module(package="a") and __protobuf__ = proto.module(package="b") you can somewhat define proto packages.

@fischor fischor added priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Aug 28, 2021
@upcFrost
Copy link

upcFrost commented Jan 10, 2023

Same question. Also, how should cross-package referencing work?

For example:

# composer.py

import proto

class Composer(proto.Message):
    given_name = proto.Field(proto.STRING, number=1)

__protobuf__ = proto.module(package="composer", manifest={"Composer"})
# song.py

import proto
import composer

__protobuf__ = proto.module(package="song", manifest={"Song"})

class Song(proto.Message):
    composer = proto.Field(composer.Composer, number=1)
# __init__.py

import composer
import song

s = song.Song(composer=composer.Composer())

This will crash with

Traceback (most recent call last):
  File "/Users/upcfrost/tmp/protoplus/__init__.py", line 3, in <module>
    s = song.Song(composer=composer.Composer())
  File "/Users/upcfrost/.conda/envs/test/lib/python3.9/site-packages/proto/message.py", line 599, in __init__
    super().__setattr__("_pb", self._meta.pb(**params))
TypeError: Parameter to MergeFrom() must be instance of same class: expected _default_package.Composer got Composer.

It does work with dicts and with composer.Composer._pb, but this is pretty awkward to always keep in mind from which package which class came. In our case, we have a set of common messages which live in a separate package

In marshal.py I can see that the rule for the imported class does not get registered (well, it does, but in the different package), and because of that it later can't be found using

rule = self._rules.get(proto_type, self._noop)
return rule.to_python(value, absent=absent)

Shouldn't this search take into account imported packages?

@parthea
Copy link
Contributor

parthea commented Nov 21, 2023

For #244 (comment), the issue has been fixed in #348

@parthea parthea added type: docs Improvement to the documentation for an API. and removed type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. priority: p3 Desirable enhancement or fix. May not be included in next release. labels Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: docs Improvement to the documentation for an API.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants