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

experiment: changing protobuf directory to match Python package name #142

Closed
wants to merge 4 commits into from

Conversation

indirectlylit
Copy link

Public-Facing Changes

None

Description

Simplify Python protobuf schema generation by matching directory structure to Python package name

@indirectlylit
Copy link
Author

I briefly tested the generated Python package locally, and this still works:

from foxglove_schemas_protobuf.CompressedImage_pb2 import CompressedImage

However I'm not entirely confident this change maintains full backward compatibility with the existing package.

@jtbandes
Copy link
Member

I see that the imports are now import "foxglove_schemas_protobuf/foo.proto"; Is that sort of naming scheme common among third-party protobuf packages? The built-in ones look like import "google/protobuf/duration.proto"; so one might expect to see import "foxglove/protobuf/foo.proto"; — but I'm wondering what is done most commonly in third-party packages?

@indirectlylit
Copy link
Author

indirectlylit commented Feb 23, 2024

No, I doubt that this is common, and is the reason I didn't make this PR originally: it ties our *.proto files to the somewhat arcane Python package naming conventions (to say nothing about the possibility of eventually adopting a global foxglove implicit namespace).

Another option might be to generate a secondary set of *.proto files specifically for the Python package? That might be very marginally cleaner than the existing sed logic.

@indirectlylit indirectlylit changed the title changing protobuf directory to match Python package name experiment: changing protobuf directory to match Python package name Feb 23, 2024
@indirectlylit indirectlylit marked this pull request as draft February 23, 2024 21:34
@indirectlylit
Copy link
Author

Another alternative might be to use https://github.com/danielgtaylor/python-betterproto which I believe provides more flexibility in specifying package names, among other things. (ref: FG-6783)

@indirectlylit
Copy link
Author

closing experimental PR

@jtbandes jtbandes deleted the protobuf-dir branch February 29, 2024 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants