JingsongLi commented on code in PR #9708:
URL: https://github.com/apache/paimon/pull/9708#discussion_r3974902962
##########
paimon-python/pypaimon/tests/multimodal_lerobot_writer_test.py:
##########
@@ -195,17 +364,150 @@ def
test_commits_multiple_completed_episodes_as_one_batch(self):
"timestamp",
"index",
"task_index",
- "task",
"observation.state",
"action",
]).to_arrow().sort_by("index").to_pylist()
self.assertEqual([0, 1], [row["episode_index"] for row in rows])
self.assertEqual([0, 0], [row["frame_index"] for row in rows])
self.assertEqual([0, 1], [row["index"] for row in rows])
self.assertEqual([0, 1], [row["task_index"] for row in rows])
- self.assertEqual(["pick", "place"], [row["task"] for row in rows])
self.assertEqual([0.0, 0.0], [row["timestamp"] for row in rows])
self.assertEqual([0.5, 0.0], [row["action"] for row in rows])
+ self.assertEqual([
+ {"task_index": 0, "task": "pick"},
+ {"task_index": 1, "task": "place"},
+ ], _catalog_rows(self.connection, "robot_data__tasks"))
+
+ def test_multiple_flushes_append_rows_and_replace_info(self):
+ writer = PaimonLeRobotWriter(
+ self.connection,
+ "multiple_flushes",
+ fps=10,
+ episodes_per_commit=1,
+ features={
+ "action": {
+ "dtype": "float32",
+ "shape": (1,),
+ "names": None,
+ },
+ },
+ )
+ for value, task in ((1.0, "pick"), (2.0, "place")):
+ writer.add_frame({
+ "action": np.array([value], dtype=np.float32),
+ "task": task,
+ })
+ writer.save_episode()
+ writer.finalize()
+
+ self.assertEqual(2, len(_catalog_rows(
+ self.connection, "multiple_flushes__episodes")))
+ self.assertEqual(2, len(_catalog_rows(
+ self.connection, "multiple_flushes__tasks")))
+ info_rows = _catalog_rows(self.connection, "multiple_flushes__info")
+ self.assertEqual(len({row["key"] for row in info_rows}),
+ len(info_rows))
+ info = {
+ row["key"]: json.loads(row["value"])
+ for row in info_rows
+ }
+ self.assertEqual(2, info["total_frames"])
+ self.assertEqual(2, info["total_episodes"])
+ self.assertEqual(2, info["total_tasks"])
+ self.assertEqual({"train": "0:2"}, info["splits"])
+
+ def test_persists_native_episode_and_global_stats(self):
+ from lerobot.datasets.compute_stats import (
Review Comment:
[P1] Guard the tests that now require the LeRobot extra
The writer now imports LeRobot statistics during construction, and this test
also imports them directly, but the test class has no dependency guard. The CI
workflow installs `pypaimon[lerobot]` only in the Python 3.10 lane, so the
3.11, 3.12, and 3.13 lanes each fail 15 writer tests with
`ImportError`/`ModuleNotFoundError`. This is already visible in the [Python
3.13
job](https://github.com/apache/paimon/actions/runs/34364056401/job/102508105228),
and I reproduced it locally; the baseline writer tests pass in the same
environment. Please conditionally skip the dependency-requiring tests when
LeRobot is unavailable, or install the extra in the relevant CI lanes, while
keeping the missing-dependency test runnable.
##########
paimon-python/pypaimon/multimodal/lerobot/writer.py:
##########
@@ -289,6 +556,12 @@ def save_episode(self):
raise ValueError("Cannot save an empty LeRobot episode.")
episode = self._episode_table()
+ episode_stats = _compute_stats(episode, self.features)
+ aggregate_stats = _lerobot_stats_functions()[0]
+ stats = (
+ aggregate_stats([self._stats, episode_stats])
Review Comment:
[P2] Aggregate numeric features without the image-name heuristic
LeRobot 0.4.4's `aggregate_stats` treats any feature name containing `image`
as an image and requires every non-count statistic to have shape `(3, 1, 1)`. A
valid numeric feature such as `observation.image_embedding` with dtype
`float32` and shape `(2,)` therefore saves its first episode successfully but
fails on the second `save_episode()` with `Shape of quantile 'min' must be
(3,1,1), but is (2,) instead`. Finalizing just the first episode and reopening
also fails at the aggregation check in `_state_from_companion_tables`. I
verified that the baseline writer accepts two episodes and resumes with this
same feature. Please aggregate/validate using the declared feature dtype, for
example with per-feature aggregation or neutral internal keys, so numeric
fields retain their existing naming support.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
