[DO NOT MERGE] feat: remove generated protobuf code from substrait-python #145
Conversation
Avoiding conflicts with generated substrait.extension protos
|
|
vbarua
changed the title
|
ACTION NEEDED Substrait follows the Conventional Commits The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification. |
There was a problem hiding this comment.
I moved all of the extension files to a different dedicated directory, to avoid a conflict with the substrait.extensions module which is where the extension.proto code gets generated into.
| from substrait.parameterized_types_pb2 import * | ||
| from substrait.plan_pb2 import * | ||
| from substrait.type_expressions_pb2 import * | ||
| from substrait.type_pb2 import * |
There was a problem hiding this comment.
I'm not clever enough to recreate whatever was happening in proto.py, but I am smart enough to re-export it all by hand. It's not as flashy, but it does the job.
There was a problem hiding this comment.
tbh, pretty sure that module was there to avoid writing out gen.proto in imports (which I was doing anyway 😆) if protos are moved to top-level substrait namespace, I don't think we need to keep this around.
| @@ -0,0 +1,3 @@ | |||
| from importlib.metadata import version | |||
|
|
|||
| substrait_version = version("vbarua-substrait-protobuf") | |||
There was a problem hiding this comment.
Instead of baking in the substrait version into substrait-python, we can just use the version of substrait-protobuf which should correspond to a spec version.
|
|
||
| __substrait_version__ = "0.79.0" | ||
| __substrait_hash__ = "92d2e75" | ||
| __minimum_substrait_version__ = "0.30.0" |
There was a problem hiding this comment.
Of these 3 versions, the only thing I've preserved is substrait_version which can be found in version.py now.
substrait_hash is optional, and we don't need to be setting it or making it available if we're making the substrait_version available.
I'm not sure what __minimum_substrait_version__ is for, but it hasn't been update in over 2 years.
There was a problem hiding this comment.
this made me think if we might have a problem if someone inadvertently installs different versions of substrait-protobuf and substrait-antlr for example... do you think substrait_version should refer exclusively to protobuf? or maybe we need some sort of an eager check to make sure all those substrait package versions line up
|
This looks like a lot of changes, but the vast majority of changes are just import path updates of to |
Generated code is packaged and available in the substrait-protobuf package