Skip to content

Improve types and bytes usage of pathlib #9016

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
srittau merged 6 commits into from
Oct 28, 2022
Merged

Improve types and bytes usage of pathlib #9016

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b89c894
Improve types and `bytes` usage of `pathlib`
sobolevn Oct 28, 2022
91633a0
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 28, 2022
6f5bb25
Fix ci
sobolevn Oct 28, 2022
b2fbfe4
Merge branch 'bytes-pathlib' of https://github.com/sobolevn/typeshed …
sobolevn Oct 28, 2022
c23edae
Address review
sobolevn Oct 28, 2022
b4c23a3
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions stdlib/pathlib.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ from _typeshed import (
OpenBinaryModeUpdating,
OpenBinaryModeWriting,
OpenTextMode,
ReadableBuffer,
Self,
StrPath,
)
Expand Down Expand Up @@ -188,16 +189,16 @@ class Path(PurePath):
def expanduser(self: Self) -> Self: ...
def read_bytes(self) -> bytes: ...
def read_text(self, encoding: str | None = ..., errors: str | None = ...) -> str: ...
def samefile(self, other_path: str | bytes | int | Path) -> bool: ...
def write_bytes(self, data: bytes) -> int: ...
def samefile(self, other_path: StrPath) -> bool: ...

@srittau srittau Oct 28, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This just calls other_path.stat(), which should also work with Path[bytes]. In case it's not a Path, it's passed on to os.stat(), which uses int | StrOrBytesPath. I therefore think the following would be correct:

Suggested change
def samefile(self, other_path: StrPath) -> bool: ...
def samefile(self, other_path: StrOrBytesPath | int) -> bool: ...

@AlexWaygood AlexWaygood Oct 28, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Path[bytes] isn't a thing — Path isn't a generic class. pathlib only makes promises to work with string paths, unlike os.path or functions in os

@srittau srittau Oct 28, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I always mix up Path and PathLike. The annotation should be correct nevertheless.

@sobolevn sobolevn Oct 28, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Here' how this method is implemented:

    def samefile(self, other_path):
        st = self.stat()
        try:
            other_st = other_path.stat()
        except AttributeError:
            other_st = self.__class__(other_path).stat()
        return os.path.samestat(st, other_st)

So, other_path can either be:

  • Something with .stat() method (assuming Path, it can be a new protocol, but I don't think it is worth it right now)
  • Something that self.__class__.__new__ accepts: StrPath (== str | PathLike[str])

But, Path is a part of StrPath, because Path is PathLike[str].
That's why StrPath seems correct to me. Am I missing something?

@srittau srittau Oct 28, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah interesting, I was looking at the 3.9 version of the implementation:

https://github.com/python/cpython/blob/b43496c01a554cf41ae654a0379efae18609ad39/Lib/pathlib.py#L1145-L1154

    def samefile(self, other_path):
        """Return whether other_path is the same or not as this file
        (as returned by os.path.samefile()).
        """
        st = self.stat()
        try:
            other_st = other_path.stat()
        except AttributeError:
            other_st = self._accessor.stat(other_path)
        return os.path.samestat(st, other_st)

No need to version guard this. Let's just use the latest version (yours).

def write_bytes(self, data: ReadableBuffer) -> int: ...
if sys.version_info >= (3, 10):
def write_text(
self, data: str, encoding: str | None = ..., errors: str | None = ..., newline: str | None = ...
) -> int: ...
else:
def write_text(self, data: str, encoding: str | None = ..., errors: str | None = ...) -> int: ...
if sys.version_info >= (3, 8):
def link_to(self, target: StrPath | bytes) -> None: ...
if (3, 8) <= sys.version_info < (3, 12):
Comment thread
AlexWaygood marked this conversation as resolved.
Outdated
def link_to(self, target: StrPath) -> None: ...
Comment thread
AlexWaygood marked this conversation as resolved.
Outdated
if sys.version_info >= (3, 12):
def walk(
self: Self, top_down: bool = ..., on_error: Callable[[OSError], object] | None = ..., follow_symlinks: bool = ...
Expand Down