-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Incorrect types are used to describe google.protobuf enums #2521
Comments
This looks reasonable. I would put the NewType in the global scope instead of within the class and give it a |
Great, so I'm going to submit PR soon. |
@JelleZijlstra I've done some refactorings in vmagamedov@fe85fcd and it turns out that there are lots of changes needed. I've managed to complete them, but it is easy to make a mistake there and it would be hard to review :( I have an idea to automatically generate all And there is a problem, enums declared on the module level can be imported into other modules, and enum value types (declared with NewType) should be imported as well. It would be odd if they remain to be |
hi - mypy-protobuf recently made updates which fixes this issue - I can run mypy-protobuf on the google That'll make them more correct - though it might be a backward compatibility painpoint for those using them. |
Let's take
google.protobuf.descriptor_pb2.FieldDescriptorProto
as an example.In Python
FieldDescriptorProto.TYPE_DOUBLE
is instance ofint
In mypy
FieldDescriptorProto.TYPE_DOUBLE
is instance ofFieldDescriptorProto.Type
In Python
FieldDescriptorProto.Type
is instance ofEnumTypeWrapper
In mypy
FieldDescriptorProto.Type
is subclass ofint
with additional methodsIn Python
FieldDescriptorProto.Type.keys()
is of typeList[str]
In mypy
FieldDescriptorProto.Type.keys()
is of typeList[bytes]
In Python
EnumTypeWrapper.items
is aninstancemethod
In mypy
EnumTypeWrapper.items
is aclassmethod
In mypy this code works:
In Python it fails:
Currently this stub looks like this:
I think it is possible to change it into this:
where
google.protobuf.internal.enum_type_wrapper.EnumTypeWrapper
looks like this:The only problem I see is how to generate unique type name for enum values, in the example above I'm using concatenation of message name and enum name:
FieldDescriptorProto_Type
.Is it ok to create PR with this fix?
The text was updated successfully, but these errors were encountered: