[Data] Surface MCAP channel metadata as a map column - #65916
Conversation
A Channel record carries a `metadata` field of type `Map<string, string>` (MCAP specification, op=0x04) holding whatever the recorder chose to record about the topic. ROS 2 writes `offered_qos_profiles` there, and rigs commonly add sensor serials or calibration identifiers. No message record reproduces it, so it was not reachable from the dataset at all: `include_metadata` added the schema and channel *ids*, but not the channel's own metadata map. Add it as a `channel_metadata` column under `include_metadata`. The column is typed explicitly as `map<string, string>` rather than left to inference. Arrow reads a Python dict as a `struct` whose fields are the union of the keys it happens to see, which has two consequences: blocks from two files recording different keys carry different types and raise ArrowInvalid on concat_tables, and a channel that never recorded a key becomes indistinguishable from one whose value is null. A map matches the specification's own type, keeps the block schema stable whatever the keys are, and concatenates across files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Marwan Sarieddine <sarieddine.marwan@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for extracting and appending MCAP channel metadata as a map<string, string> column (named channel_metadata) when reading MCAP files via ray.data.read_mcap. This ensures that channel-level metadata, such as ROS 2 QoS profiles or sensor serials, is preserved and accessible across concatenated blocks. The review feedback suggests two key improvements: handling fallback pandas DataFrame blocks to prevent the metadata column from being silently omitted, and defensively handling cases where channel.metadata might be None or missing.
| if values is None or not isinstance(block, pa.Table): | ||
| # `include_metadata=False`, or a pandas block, which | ||
| # `DelegatingBlockBuilder` produces for row values Arrow cannot hold. | ||
| return block |
There was a problem hiding this comment.
When DelegatingBlockBuilder falls back to producing a pandas.DataFrame (e.g., due to row values that Arrow cannot represent), the channel_metadata column is currently silently omitted because the function returns early for non-pa.Table blocks.
To ensure schema consistency and prevent the column from being dropped when falling back to Pandas, we should also handle pandas.DataFrame blocks by appending the metadata as a column of dictionaries.
| if values is None or not isinstance(block, pa.Table): | |
| # `include_metadata=False`, or a pandas block, which | |
| # `DelegatingBlockBuilder` produces for row values Arrow cannot hold. | |
| return block | |
| if values is None: | |
| return block | |
| import pyarrow as pa | |
| if isinstance(block, pa.Table): | |
| column = pa.array(values, type=pa.map_(pa.string(), pa.string())) | |
| return block.append_column(CHANNEL_METADATA_COLUMN, column) | |
| import pandas as pd | |
| if isinstance(block, pd.DataFrame): | |
| block[CHANNEL_METADATA_COLUMN] = [dict(v) for v in values] | |
| return block | |
| return block |
| if channel_metadata is not None: | ||
| channel_metadata.append(list(channel.metadata.items())) |
There was a problem hiding this comment.
To be defensive against malformed MCAP files or custom channel implementations where channel.metadata might be None or missing, we should safely handle the case where channel.metadata is not populated.
| if channel_metadata is not None: | |
| channel_metadata.append(list(channel.metadata.items())) | |
| if channel_metadata is not None: | |
| metadata = channel.metadata or {} | |
| channel_metadata.append(list(metadata.items())) |
1 participant
Description
A Channel record carries a
metadatafield of typeMap<string, string>— MCAP specification, op=0x04, "Metadata about this channel" — holding whatever the recorder chose to record about the topic. ROS 2 writesoffered_qos_profilesthere, and rigs commonly add sensor serials or calibration identifiers.No message record reproduces it, so today it is not reachable from the dataset at all.
include_metadata=Trueaddschannel_id,message_encoding,schema_name,schema_encodingandschema_data— the schema and the channel's id, but not the channel's own metadata map. A caller who needs it has to re-open the file with themcaplibrary alongside the Ray Data read.This adds a
channel_metadatacolumn under the existinginclude_metadataflag.The column is typed, not inferred
Arrow reads a Python dict as a
structwhose fields are the union of the keys it happens to see. Two consequences, both verified:promote_options="permissive"gets past it).None.A
map<string, string>matches the specification's own type, keeps the block schema stable whatever the keys are, and concatenates across files without promotion. It is built explicitly in_add_channel_metadatarather than added in_message_to_dict, since the builder would otherwise infer the struct.Verified on a real recording
A FAST-LIVO ROS 2 recording (Livox IMU + lidar + two compressed camera streams):
Every rosbag2-produced MCAP populates this field, so this is the common case rather than a corner one.
Why this is not a duplicate
I checked issues and PRs for
mcap,channel_metadataandmetadatain all states.schema_data) — closest overlap, and it touches the same two files. Its diff contains no reference tochannel_metadataorchannel.metadata; it deduplicates a column that already exists rather than adding one. It will conflict textually with this PR on theyieldat the end of_read_stream, where both wrapbuilder.build(). The two compose —_dictionary_encode_schema_data(_add_channel_metadata(builder.build(), channel_metadata))— and whichever lands second, I'll rebase.batch_size) modifies__init__and_read_stream's iteration, not the row contents.mcapin CI) is test infrastructure.Compatibility
include_metadata=Trueis the default, so this adds a column to the default output. It is additive — existing code selecting named columns is unaffected — but code asserting an exact schema will see the new column.read_mcapis@PublicAPI(stability="alpha"). Happy to move it behind a separate flag if maintainers prefer.Tests
21 passed, 1 failed. The failure is
test_read_mcap_invalid_time_range, which fails identically on clean master — its regex omits the values the error message interpolates, so it can never match. #65912 fixes it; it is untouched here. (It is not caught in CI becausemcapis in no requirements file, which is what #65914 addresses.)Four new tests: the column carries the channel's metadata and is typed
map;include_metadata=Falseomits it; blocks from files with different metadata keys still concatenate (the reason for map over struct); and a channel recording no metadata yields an empty map rather than failing.Lint:
black,ruffclean on all three files.AI assistance
AI assistance (Claude) was used for this change. I reviewed every changed line and ran the tests locally.