Skip to content

feat(runfiles): implement runfiles.Path io methods#3716

Open
rickeylev wants to merge 9 commits intobazel-contrib:mainfrom
rickeylev:runfiles.pathlib.io
Open

feat(runfiles): implement runfiles.Path io methods#3716
rickeylev wants to merge 9 commits intobazel-contrib:mainfrom
rickeylev:runfiles.pathlib.io

Conversation

@rickeylev
Copy link
Copy Markdown
Collaborator

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

@rickeylev rickeylev requested a review from aignas as a code owner April 18, 2026 23:11
Copy link
Copy Markdown
Contributor

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

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 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.

Comment thread python/runfiles/runfiles.py Outdated
runfiles: Optional["Runfiles"] = None,
source_repo: Optional[str] = None,
) -> "Path":
) -> "Self":
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.

medium

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.

Suggested change
) -> "Self":
) -> "Path":

Comment thread python/runfiles/runfiles.py Outdated
Comment thread python/runfiles/runfiles.py Outdated
Comment thread python/runfiles/runfiles.py Outdated
Comment thread python/runfiles/runfiles.py Outdated
Comment thread python/runfiles/runfiles.py Outdated
Comment thread python/runfiles/runfiles.py Outdated
Comment thread python/runfiles/runfiles.py Outdated
Comment thread python/runfiles/runfiles.py Outdated
Comment thread python/runfiles/runfiles.py Outdated
@aignas
Copy link
Copy Markdown
Collaborator

aignas commented Apr 19, 2026

The Self type annotation was only introduced in 3.13, this should be made compatible with lower Python versions too. :)

@rickeylev rickeylev force-pushed the runfiles.pathlib.io branch from 6e5df44 to efb589b Compare April 20, 2026 02:20
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.

runfiles: add pathlib.Path or importlib.Traversable API

2 participants