Skip to content

Accept pathlib.Path in checkpoint I/O helpers - #2193

Open
ethantsliu wants to merge 1 commit into
NovaSky-AI:mainfrom
ethantsliu:fix/checkpoint-pathlike
Open

Accept pathlib.Path in checkpoint I/O helpers#2193
ethantsliu wants to merge 1 commit into
NovaSky-AI:mainfrom
ethantsliu:fix/checkpoint-pathlike

Conversation

@ethantsliu

@ethantsliu ethantsliu commented Sep 9, 2026

Copy link
Copy Markdown

Accept pathlib.Path in checkpoint I/O helpers

Checkpoint I/O helpers fail on pathlib.Path inputs because cloud-path detection expects a string. Directory downloads also pass local Path destinations into fsspec code that expects strings.

Accept str | os.PathLike[str] for local paths and normalize them with os.fspath(). Directory context managers still yield strings, and cloud URIs are still supplied as strings.

The focused filesystem tests pass (4 tests, including 2 new regression tests). Both new tests fail against the original implementation. Cloud downloads are tested with an in-memory filesystem.

Part of #252.


Note

Low Risk
Behavior-preserving type widening and string normalization for local paths; cloud URI handling is unchanged aside from safer local-path coercion.

Overview
Checkpoint and filesystem helpers in io.py now accept str | os.PathLike[str] for local paths instead of strings only. Each entry point normalizes inputs with os.fspath() so cloud URI detection no longer breaks on pathlib.Path, and fsspec/local os.makedirs always receive string paths. Cloud source/destination URIs stay typed as str.

Context managers such as local_work_dir, local_read_dir, and local_read_files still yield strings after normalization. Coverage adds regressions for Path with local work/read flows and for download_directory into a Path destination using an in-memory fsspec filesystem.

Reviewed by Cursor Bugbot for commit 8aff1ab. Bugbot is set up for automated code reviews on this repo. Configure here.

Signed-off-by: Ethan Liu <ethantsliu@gmail.com>
@greptile-apps

greptile-apps Bot commented Sep 9, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

The PR appears safe to merge with no actionable defects identified.

PathLike values are normalized before string-specific operations, existing string behavior remains intact, and the new tests exercise the intended local and directory-download paths.

Important Files Changed

Filename Overview
skyrl/backends/skyrl_train/utils/io/io.py Consistently normalizes PathLike inputs before existing string and fsspec operations without changing behavior for current string callers.
tests/backends/skyrl_train/utils/test_local_read_files.py Adds focused regression tests covering Path inputs for local context managers and directory download destinations.

Reviews (1): Last reviewed commit: "Accept pathlib.Path in checkpoint I/O he..." | Re-trigger Greptile

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the file I/O utilities in skyrl/backends/skyrl_train/utils/io/io.py to support both str and os.PathLike[str] (such as pathlib.Path objects) for local filesystem paths. This is achieved by updating the type annotations and using os.fspath() to normalize the path arguments before processing. Additionally, corresponding unit tests have been added to verify this new behavior. There are no review comments, and I have no additional feedback to provide.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant