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

feat: getter for message classes #1325

Closed
wants to merge 3 commits into from
Closed

feat: getter for message classes #1325

wants to merge 3 commits into from

Conversation

magi-2
Copy link

@magi-2 magi-2 commented Jan 30, 2025

Changelog

Improves the reader API for accessing protobuf messages which are generated from the Schema objects within the DecoderFactory.

Docs

None

Description

I am using protarrow and found myself having to search for the message class objects in the DecoderFactory for the example seen below. Note that, in my current project set up, I do not have the proto message classes as a submodule / dependency. Given that mcap stores the schemas themselves, that this feature adds a lot of value so that anyone can read the logs without having to include the pb messages in their project.

import protarrow

my_protos = [
    MyProto(name="foo", id=1, values=[1, 2, 4]),
    MyProto(name="bar", id=2, values=[3, 4, 5]),
]

table = protarrow.messages_to_table(my_protos, MyProto)

The only way to access these currently is by accessing 'private' class members.

msg = reader._decoder_factories[0]._types[schema.id]
table = protarrow.messages_to_table(my_protos, msg)

The change here is a quick fix but I imagine that this can be made more ergonomic with changes to the reader class itself (python/mcap-protobuf-support/mcap_protobuf/reader.py)

I did no tests as this is very straight forward. This PR does not have to be merged though I think it's a perfect example as to how this API can be improved.

BeforeAfter
msg_before = reader._decoder_factories[0]._types[schema.id]
msg_after = reader._decoder_factories[0].pb_message_from_schema(schema)

If this feature is of interest, let me know and we can discuss what methods could be of interest. It would be nice if the reader itself offered this API. That being said, for the reader base-class I am not sure if similar changes make sense.

@magi-2 magi-2 closed this by deleting the head repository Jan 31, 2025
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.

1 participant