Repository navigation
fix(lerobot): refuse non-finite FPS values in meta/info.json - #313
Conversation
The LeRobot metadata boundary checked that fps is numeric and positive but not finite, so NaN and +/-inf passed validation and failed later with unrelated errors (no meta/episodes parquet files found, or a raw int() conversion error). Mirror the TransformConfig boundary: require fps to be finite as well as numeric and positive, and name the bad value with a complete rule in the error. Regression coverage parametrizes NaN, +inf, -inf, bool, missing, nonnumeric, zero, and negative fps, asserting refusal before any episode metadata discovery or output is written. Fixes Hebbian-Robotics#299
kstonekuan
left a comment
There was a problem hiding this comment.
LGTM, merging.
Validated on your branch: ruff check, ruff format --check, ty check clean, 1260 passed / 6 skipped.
Also checked the tests can detect the fix being removed. Dropped the math.isfinite clause and the nan and positive-infinity cases failed, the other six still passed. That's the right split: -inf was already caught by <= 0, so nan and +inf are the actual new coverage and your parametrization makes that visible instead of hiding it behind one green test.
"finite and positive" matches what _camera_motion.py:59 and _frame_statistics.py:98 already say, so the message reads like the rest of the repo.
One thing, and it's an invitation not a rule: the good first issue pool is where newcomers land, and you're three PRs in now, so it's worth leaving those for people who need somewhere to start. #320 (parse LeRobot metadata into a typed archive model) is the natural next step from this one, and it's where the _SourceArchive TypedDict you just validated a field of gets turned into something that can't hold a bad fps in the first place.
Or the higher-leverage version: you already filed #287 off Egocentric-10K, and that report was more useful than any of these fixes. More of that, pointing HFlow at real corpora and telling us what breaks, is worth more to us than the starter queue.
Summary
Fixes #299.
The LeRobot metadata boundary in
_ensure_source_archivevalidated thatfpsfrommeta/info.jsonis numeric and positive, but not finite — sofloat("nan")andfloat("inf")passed validation and only failed later with misleading errors (no meta/episodes parquet files found, or a rawint()conversion failure in episode conversion).This change mirrors the existing
TransformConfig.gop_secondspattern (math.isfinite) at the metadata boundary:fpsmust now be finite as well as numeric and positive.ValueErrornames the offending value and states the complete rule:FPS must be finite and positive.Tests
Parametrized metadata-boundary regressions in
tests/test_lerobot_converter.py:NaN,+inf, and-infare refused as invalid FPS (new behavior).30) is still accepted through discovery._hf_treeepisode discovery never runs and nometa/episodesoutput is created.No new dependency and no converter-version bump.
Validation
uv run ruff check --fix— cleanuv run ruff format— cleanuv run ty check— cleanuv run pytest -q tests/test_lerobot_converter.py— 18 passedtests+packages/hflow-server/testspertestpaths): 1255 passed, 11 skipped, 0 failed