JingsongLi commented on code in PR #10483: URL: https://github.com/apache/paimon/pull/10483#discussion_r4313979408
########## paimon-python/pypaimon/multimodal/lerobot/ray_loader.py: ########## @@ -0,0 +1,168 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Ray workers prepare LeRobot files; only the driver commits snapshots.""" + +import logging +from pathlib import Path + +from pypaimon.multimodal.lerobot.loader import ( + _abort_messages, _commit_messages, _prepare_dataset, +) +from pypaimon.multimodal.lerobot.source import ( + _RemoteLeRobotDataset, _resolved_source, +) + + +_CANCEL_DRAIN_SECONDS = 1.0 + + +def _require_ray(): + try: + import ray + except ImportError as error: + raise ImportError("Parallel import requires 'pypaimon[ray,lerobot]'.") from error + if not ray.is_initialized(): + raise RuntimeError("Call ray.init() before using parallelism > 1.") + return ray + + +def _ray_source_uri(source): + if isinstance(source, str) and "://" in source: + return source + if isinstance(source, Path) or isinstance(source, str) and Path(source).is_dir(): Review Comment: [P2] Expand string source paths before checking whether they are directories The existing serial resolver expands '~' (and strips surrounding whitespace), but this admission check tests the unexpanded string. A valid shared-home path such as '~/datasets/robot' is therefore rejected as a Hub ID when parallelism is enabled. I reproduced this through the public API with a real four-file, path-backed-image dataset on Ray 2.54: the tilde string with parallelism=1 commits and reads back all 8 frames; Path(the_same_tilde_string) with parallelism=2 also returns 8 frames; changing only that Path argument to the equivalent string with parallelism=2 raises 'Parallel import requires a FileIO URI or shared directory, not a Hub repo_id' before dispatch. Please strip/expand local string paths before is_dir(), then canonicalize them to a file URI, and add the string form to the compatibility regression. -- 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]
