danielgtaylor / python-betterproto

Clean, modern, Python 3.6+ code generator & library for Protobuf 3 and async gRPC
MIT License
1.44k stars 196 forks source link

Imports can conflity with typing types #580

Open imcdo opened 4 weeks ago

imcdo commented 4 weeks ago

Summary

K8s List type proto conflicts with the typing List type

Reproduction Steps

use https://github.com/kubernetes/kubernetes/blob/c6b5191c37f939d2d61e76de222a96ae5f5d9558/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/generated.proto#L456 and try to generate proto for it.

Expected Results

The List type in the proto collide with the imported typing List. Resulting in

TypeError: type 'List' is not subscriptable

Actual Results

The typing import is either imported via alias or accessed via the typing module:

import typing
...
list_of_things: typing.List[abc]

or

from typing import list as ListType
...
list_of_things: ListType[abc]

or even support more modern conventions for 3.9> support with no typing import:

list_of_things: list[abc]

System Information

libprotoc 3.9.0 Python 3.11.9 Name: betterproto Version: 2.0.0b6 Summary: A better Protobuf / gRPC generator & library Home-page: https://github.com/danielgtaylor/python-betterproto Author: Daniel G. Taylor Author-email: danielgtaylor@gmail.com License: MIT Location: /Users/ianmcdonald/git/cc-structs/build/venv/lib/python3.11/site-packages Requires: grpclib, python-dateutil Required-by:

Checklist

Gobot1234 commented 4 weeks ago

I think this would be better as a command line option tbh. i.e --typing-import-type=full|short|none where full uses this pr's method short is the default which is the way it currently is and the none method uses 3.10+ style optional and pep 585 builtins

imcdo commented 4 weeks ago

yeah for sure. I do think in general some sort of validation of the final namespace to check for these kinds of issues might be worth wile as well then. We dont want be able to alert when the flag should be used (in this case would be nice to have some warning to indicate that the flag should be used).