Add try_from_path_like wrapper by youknowone · Pull Request #6359 · RustPython/RustPython

youknowone · 2025-12-08T19:37:11Z

Summary by CodeRabbit

Release Notes

Refactor
- Improved internal path handling and error messaging for path-like object conversion.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-08T19:37:18Z

Walkthrough

Refactored filesystem path-like object handling in RustPython by introducing try_from_path_like as a public wrapper method in FsPath and making try_from accept a customizable error message. Added a new try_from_fspath helper in OsPath. Updated callers across file I/O and OS modules to use the appropriate conversion methods.

Changes

Cohort / File(s)	Summary
Path-like conversion API refactoring `crates/vm/src/function/fspath.rs`	Added new `try_from_path_like(obj, check_for_nul, vm)` public method with fixed error message. Changed `try_from` signature to accept customizable `msg` parameter. Reworked error handling with dynamic closure. Updated `TryFromObject::try_from_object` to call `try_from_path_like` instead of `try_from`.
OsPath path-like conversion helpers `crates/vm/src/ospath.rs`	Added new `try_from_fspath(obj, vm)` crate-local helper method. Updated `OsPath::try_from_object` to delegate to `FsPath::try_from` with expanded error message "should be string, bytes, os.PathLike or integer".
File I/O path construction `crates/vm/src/stdlib/io.rs`	Replaced `OsPath::try_from_object(vm, name.clone())?` with `OsPath::try_from_fspath(name.clone(), vm)?` in FileIO initialization.
OS module path-like conversion `crates/vm/src/stdlib/os.rs`	Replaced `FsPath::try_from(path, false, vm)` with `FsPath::try_from_path_like(path, false, vm)` in fspath functionality.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Areas requiring extra attention:

Verify all call sites of the modified FsPath::try_from signature have been updated throughout the codebase
Confirm error message changes in OsPath::try_from_object are appropriate for all affected code paths
Check that the new try_from_fspath helper properly delegates error handling and matches expected behavior in file I/O operations
Ensure consistency between try_from_path_like wrapper and try_from with custom message implementations

Poem

🐰 Path conversions now flow with grace,
Error messages find their place,
New helpers guide the filesystem way,
Better errors brighten the day!
Refactored code hops along so neat,
Making path handling complete! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'fspath try from' is vague and does not clearly convey what changes are being made. It lacks specific details about the purpose or scope of modifications.	Use a more descriptive title that explains the changes, such as 'Refactor FsPath conversion API to support customizable error messages' or 'Add try_from_path_like wrapper and customize FsPath error handling'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bafaa1a and 87147e1.

⛔ Files ignored due to path filters (1)

Lib/test/test_os.py is excluded by !Lib/**

📒 Files selected for processing (4)

crates/vm/src/function/fspath.rs (3 hunks)
crates/vm/src/ospath.rs (2 hunks)
crates/vm/src/stdlib/io.rs (1 hunks)
crates/vm/src/stdlib/os.rs (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

crates/vm/src/stdlib/os.rs
crates/vm/src/ospath.rs
crates/vm/src/function/fspath.rs
crates/vm/src/stdlib/io.rs

🧠 Learnings (1)

📓 Common learnings

Learnt from: moreal
Repo: RustPython/RustPython PR: 5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration using #ifdef checks rather than providing fallback values for other platforms.

🧬 Code graph analysis (4)

crates/vm/src/stdlib/os.rs (1)

crates/vm/src/function/fspath.rs (1)

try_from_path_like (17-28)

crates/vm/src/ospath.rs (2)

crates/vm/src/function/fspath.rs (3)

obj (141-141)

try_from_path_like (17-28)

try_from (31-80)

crates/vm/src/stdlib/os.rs (2)

obj (900-900)

fspath (1024-1026)

crates/vm/src/function/fspath.rs (1)

crates/vm/src/stdlib/os.rs (3)

obj (900-900)

name (501-503)

__fspath__ (634-636)

crates/vm/src/stdlib/io.rs (1)

crates/vm/src/ospath.rs (1)

try_from_fspath (61-64)

🔇 Additional comments (8)

crates/vm/src/function/fspath.rs (4)

17-28: LGTM! Clean wrapper for common path-like conversion.

This provides a sensible default error message for the typical use case while allowing callers to customize the message via try_from when needed.

31-36: LGTM! Flexible API for customizable error messages.

Adding the msg parameter enables different callers (e.g., os.fspath vs open()) to provide context-appropriate error messages.

62-71: LGTM! Proper handling of __fspath__ being explicitly set to None.

The explicit None check (lines 69-71) correctly handles edge cases where __fspath__ exists but is set to None, treating it as if the object doesn't support the path-like protocol.

149-149: LGTM!

Correctly delegates to the new wrapper method.

crates/vm/src/ospath.rs (2)

58-64: LGTM! Well-documented helper for open()-style path conversion.

The docstring clearly explains when to use this method vs try_from_object. The distinction between the two error messages is appropriate since try_from_object (line 104) also supports integer file descriptors, while try_from_fspath is specifically for path-like objects.

101-106: LGTM!

The error message correctly includes "or integer" since OsPath::try_from_object is used in contexts where file descriptors are valid inputs (the caller falls back to OsPathOrFd handling).

crates/vm/src/stdlib/os.rs (1)

1024-1026: LGTM!

Correctly uses the new wrapper with check_for_nul: false, which is appropriate for os.fspath since NUL byte checking should happen at the point of use, not during the path-like conversion.

crates/vm/src/stdlib/io.rs (1)

4343-4343: LGTM!

Correctly uses the new try_from_fspath helper which provides the appropriate error message for file open operations: "expected str, bytes or os.PathLike object" rather than "should be string, bytes, os.PathLike or integer". This aligns with CPython's behavior for open().

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

fspath try from

87147e1

youknowone marked this pull request as ready for review December 8, 2025 20:09

youknowone changed the title ~~fspath try from~~ Add try_from_path_like wrapper Dec 8, 2025

youknowone merged commit bb4e30a into RustPython:main Dec 8, 2025
13 checks passed

youknowone deleted the fspath branch December 8, 2025 20:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add try_from_path_like wrapper#6359

Add try_from_path_like wrapper#6359
youknowone merged 1 commit intoRustPython:mainfrom
youknowone:fspath

youknowone commented Dec 8, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 8, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

youknowone commented Dec 8, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai bot commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

youknowone commented Dec 8, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 8, 2025 •

edited

Loading