feat(runfiles): implement runfiles.Path io methods#3716
feat(runfiles): implement runfiles.Path io methods#3716rickeylev wants to merge 2 commits intobazel-contrib:mainfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the runfiles.Path class to inherit from pathlib.Path instead of pathlib.PurePath, enabling full I/O support such as stat, exists, and file reading, and adds comprehensive tests for these features. The review feedback identifies that the "Self" type hint is used in multiple method signatures without being imported or defined, which could cause type-checking failures; it is recommended to use "Path" for these forward references to ensure compatibility across Python versions.
| runfiles: Optional["Runfiles"] = None, | ||
| source_repo: Optional[str] = None, | ||
| ) -> "Path": | ||
| ) -> "Self": |
There was a problem hiding this comment.
The type hint "Self" is used here and in several other methods throughout the class, but Self is not imported from typing (available in Python 3.11+) nor is it a defined class. This will cause issues with type checkers. Since this library maintains compatibility with older Python versions, you should use "Path" instead of "Self" for forward references to the current class.
| ) -> "Self": | |
| ) -> "Path": |
| # to self._as_path() because super().resolve() creates intermediate objects | ||
| # that would otherwise crash during internal stat() calls. | ||
| # override | ||
| def resolve(self, strict: bool = False) -> "Self": |
There was a problem hiding this comment.
Replace "Self" with "Path" to ensure compatibility with type checkers, as Self is not defined in this scope.
| def resolve(self, strict: bool = False) -> "Self": | |
| def resolve(self, strict: bool = False) -> "Path": |
| ) | ||
|
|
||
| # override | ||
| def absolute(self) -> "Self": |
There was a problem hiding this comment.
Replace "Self" with "Path" for the return type hint.
| def absolute(self) -> "Self": | |
| def absolute(self) -> "Path": |
|
|
||
| def with_segments(self, *pathsegments: Union[str, os.PathLike]) -> "Path": | ||
| # override | ||
| def with_segments(self, *pathsegments: Union[str, os.PathLike]) -> "Self": |
There was a problem hiding this comment.
Replace "Self" with "Path" for the return type hint.
| def with_segments(self, *pathsegments: Union[str, os.PathLike]) -> "Self": | |
| def with_segments(self, *pathsegments: Union[str, os.PathLike]) -> "Path": |
|
|
||
| def _make_child(self, args: Tuple[str, ...]) -> "Path": | ||
| # override | ||
| def _make_child(self, args: Tuple[str, ...]) -> "Self": |
There was a problem hiding this comment.
Replace "Self" with "Path" for the return type hint.
| def _make_child(self, args: Tuple[str, ...]) -> "Self": | |
| def _make_child(self, args: Tuple[str, ...]) -> "Path": |
|
|
||
| def with_suffix(self, suffix: str) -> "Path": | ||
| # override | ||
| def with_suffix(self, suffix: str) -> "Self": |
There was a problem hiding this comment.
Replace "Self" with "Path" for the return type hint.
| def with_suffix(self, suffix: str) -> "Self": | |
| def with_suffix(self, suffix: str) -> "Path": |
| return self._as_path().read_text(encoding=encoding, errors=errors) | ||
|
|
||
| # override | ||
| def iterdir(self) -> Generator["Self", None, None]: |
There was a problem hiding this comment.
Replace "Self" with "Path" in the Generator type hint.
| def iterdir(self) -> Generator["Self", None, None]: | |
| def iterdir(self) -> Generator["Path", None, None]: |
| yield self / p.name | ||
|
|
||
| # override | ||
| def glob(self, pattern: str) -> Generator["Self", None, None]: |
There was a problem hiding this comment.
Replace "Self" with "Path" in the Generator type hint.
| def glob(self, pattern: str) -> Generator["Self", None, None]: | |
| def glob(self, pattern: str) -> Generator["Path", None, None]: |
| yield self / p.relative_to(resolved) | ||
|
|
||
| # override | ||
| def rglob(self, pattern: str) -> Generator["Self", None, None]: |
There was a problem hiding this comment.
Replace "Self" with "Path" in the Generator type hint.
| def rglob(self, pattern: str) -> Generator["Self", None, None]: | |
| def rglob(self, pattern: str) -> Generator["Path", None, None]: |
| return str(self) | ||
|
|
||
| def runfiles_root(self) -> "Path": | ||
| def runfiles_root(self) -> "Self": |
There was a problem hiding this comment.
Replace "Self" with "Path" for the return type hint.
| def runfiles_root(self) -> "Self": | |
| def runfiles_root(self) -> "Path": |
|
The Self type annotation was only introduced in 3.13, this should be made compatible with lower Python versions too. :) |
2 participants
The runfiles.Path class is currently a PurePath subclass, which means
it only does string manipulation for constructing paths. This change
adds functionality to actually access files.
Fixes #3296