Giter Club home page Giter Club logo

Comments (5)

nipunn1313 avatar nipunn1313 commented on August 22, 2024 1

@forestbelton - mind taking a look at #112 to see if it handles the case you've brought up here (There are tests which should help illustrate).

from mypy-protobuf.

nipunn1313 avatar nipunn1313 commented on August 22, 2024

Interesting. Thanks for the report.

Do you know what line of your source file you see this error at? What is going on in your source leading to this? Is it at the time of import?

The .py code you pasted seems to indicate that Foo is a Message here. It would also be helpful to have the equivalent .pyi code pasted to debug.

These errors:

generated/python/foo_pb2.pyi:123: error: Function "Foo" is not valid as a type
generated/python/foo_pb2.pyi:123: note: Perhaps you need "Callable[...]" or a callback protocol?

seem to indicate that something in your code is using Foo as a function rather than as a message?

If you have a repro of the situation that you are willing to share, that'll help (ideally minimal).

Hopefully this is enough information to help your debugging!

from mypy-protobuf.

forestbelton avatar forestbelton commented on August 22, 2024

I believe I've tracked down the issue. It appears there are two different errors that arise as a result of the same underlying problem — when an instance member/method has the same name as an existing type.

Here is one way to trigger an error by using an enum:

enum AThing {
    Unknown = 0;
    A = 1;
    B = 2;
}

message Foo {
    AThing AThing = 1;
}

This results in the following generated .pyi file:

# definition of AThing
# ...

class Foo(google___protobuf___message___Message):
    DESCRIPTOR: google___protobuf___descriptor___Descriptor = ...
    AThing = ... # type: AThing

    def __init__(self,
        *,
        AThing : typing___Optional[AThing] = None,
        ) -> None: ...
    if sys.version_info >= (3,):
        @classmethod
        def FromString(cls, s: builtin___bytes) -> Foo: ...
    else:
        @classmethod
        def FromString(cls, s: typing___Union[builtin___bytes, builtin___buffer, builtin___unicode]) -> Foo: ...
    def MergeFrom(self, other_msg: google___protobuf___message___Message) -> None: ...
    def CopyFrom(self, other_msg: google___protobuf___message___Message) -> None: ...
    if sys.version_info >= (3,):
        def ClearField(self, field_name: typing_extensions___Literal[u"AThing"]) -> None: ...
    else:
        def ClearField(self, field_name: typing_extensions___Literal[u"AThing",b"AThing"]) -> None: ...

And attempts to use Foo result in the following error:

from foo_pb2 import AThing, Foo

x: Foo = Foo(AThing=AThing.A)
test/foo_pb2.pyi:60: error: Variable "foo_pb2.Foo.AThing" is not valid as a type
Found 1 error in 1 file (checked 1 source file)

Per python/mypy#1775, this can be fixed by adding an alias for AThing and referring to it in Foo like so:

# definition of AThing
# ...

_AThing = AThing

class Foo(google___protobuf___message___Message):
    DESCRIPTOR: google___protobuf___descriptor___Descriptor = ...
    AThing = ... # type: _AThing

    def __init__(self,
        *,
        AThing : typing___Optional[_AThing] = None,
        ) -> None: ...
    if sys.version_info >= (3,):
        @classmethod
        def FromString(cls, s: builtin___bytes) -> Foo: ...
    else:
        @classmethod
        def FromString(cls, s: typing___Union[builtin___bytes, builtin___buffer, builtin___unicode]) -> Foo: ...
    def MergeFrom(self, other_msg: google___protobuf___message___Message) -> None: ...
    def CopyFrom(self, other_msg: google___protobuf___message___Message) -> None: ...
    if sys.version_info >= (3,):
        def ClearField(self, field_name: typing_extensions___Literal[u"AThing"]) -> None: ...
    else:
        def ClearField(self, field_name: typing_extensions___Literal[u"AThing",b"AThing"]) -> None: ...

The second way is similar, using a nested Message instead of an enum:

message Bar {
    int64 X = 1;
}

message Foo {
    Bar Bar = 1;
}

which results in the following .pyi:

# definition of Bar
# ...

class Foo(google___protobuf___message___Message):
    DESCRIPTOR: google___protobuf___descriptor___Descriptor = ...

    @property
    def Bar(self) -> Bar: ...

    def __init__(self,
        *,
        Bar : typing___Optional[Bar] = None,
        ) -> None: ...
    if sys.version_info >= (3,):
        @classmethod
        def FromString(cls, s: builtin___bytes) -> Foo: ...
    else:
        @classmethod
        def FromString(cls, s: typing___Union[builtin___bytes, builtin___buffer, builtin___unicode]) -> Foo: ...
    def MergeFrom(self, other_msg: google___protobuf___message___Message) -> None: ...
    def CopyFrom(self, other_msg: google___protobuf___message___Message) -> None: ...
    if sys.version_info >= (3,):
        def HasField(self, field_name: typing_extensions___Literal[u"Bar"]) -> builtin___bool: ...
        def ClearField(self, field_name: typing_extensions___Literal[u"Bar"]) -> None: ...
    else:
        def HasField(self, field_name: typing_extensions___Literal[u"Bar",b"Bar"]) -> builtin___bool: ...
        def ClearField(self, field_name: typing_extensions___Literal[u"Bar",b"Bar"]) -> None: ...

and the following test case reproduces the error message from my original comment:

from foo_pb2 import Bar, Foo

x: Foo = Foo(Bar=Bar(X=123))

This can also be fixed by defining an alias like _Bar = Bar and referring to _Bar in the definition of Foo.

While using a field that has the same name as a type is admittedly not a great pattern, unfortunately we are stuck with it since we are already using these types in production.

Is there any workaround we could do here?

from mypy-protobuf.

nipunn1313 avatar nipunn1313 commented on August 22, 2024

yeah - this seems like a totally reasonable concern - that may be avoidable by autogenerating an alias. We already generate some aliases like typing_extensions__Literal for mangling - so this seems like a reasonable strategy. Poking around with it to see what we can get working.

Great bugreport and analysis

from mypy-protobuf.

forestbelton avatar forestbelton commented on August 22, 2024

@nipunn1313, #112 looks like it will cover these above cases. Thanks for the quick turnaround!

from mypy-protobuf.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.