Skip to content

LeRobot: the data_path and video_path templates escape the info.json validation boundary #471

Description

@chiruu12

_parse_dataset_information is documented as the one place the external document is inspected:

Every supported field is validated here, once, so that nothing after this boundary inspects the external document again.

fps and features hold to that. The two path templates do not. They are checked for being non-empty strings and nothing else:

data_path_template = dataset_information.get("data_path")
if not isinstance(data_path_template, str) or not data_path_template.strip():
    raise ValueError("LeRobot meta/info.json must define a non-empty data_path template")

They are then handed to str.format about five hundred lines later, at _convert_episode line 1118 and line 1209, with chunk_index, file_index, video_key and camera_key. A template that names anything else, or that is not a well-formed format string, passes the boundary and raises out of the conversion instead.

Measured

Each of these is accepted by _parse_dataset_information and fails at the format call:

data_path raised
data/{episode_index:03d}/f.parquet KeyError: 'episode_index'
data/{chunk_index:03d/f.parquet ValueError: unmatched '{' in format spec
data/{chunk_index:qq}/f.parquet ValueError: Invalid format specifier 'qq' for object of type 'int'
data/{0}/f.parquet IndexError: Replacement index 0 out of range for positional args tuple
data/}chunk/f.parquet ValueError: Single '}' encountered in format string

None of them names the file or the field.

What it costs

The boundary is at line 691. Between it and the first format, the import lists meta/episodes (699), downloads every episode metadata parquet (706), and reads them through DuckDB. So the refusal lands after the metadata download rather than before it, and lands as a bare KeyError rather than as the "LeRobot meta/info.json ..." message every other invalid field produces.

The unknown-placeholder row is the one likely to be met by accident: a template that indexes episodes rather than chunk and file is a plausible thing to find in a repository that is not the v3 layout the importer expects.

Suggested fix

Format both templates once at the boundary with the four field names the converter supplies, and refuse the ones that raise, in the same shape as the existing messages:

LeRobot meta/info.json has an invalid data_path template 'data/{episode_index:03d}/f.parquet': unknown field 'episode_index'

That keeps the documented invariant true and moves five late failures to the place the docstring says they belong. The refusal tests added in #415 pin the non-empty case for both templates, so this would extend those rather than add a new file.

Happy to send a PR if you want it.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workinghelp wantedExtra attention is needed

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions