Skip to content

Improve types and bytes usage of pathlib - #9016

Merged
srittau merged 6 commits into
python:masterfrom
sobolevn:bytes-pathlib
Oct 28, 2022
Merged

Improve types and bytes usage of pathlib#9016
srittau merged 6 commits into
python:masterfrom
sobolevn:bytes-pathlib

Conversation

@sobolevn

Copy link
Copy Markdown
Member

Notes:

Source: https://github.com/python/cpython/blob/main/Lib/pathlib.py
Refs #9006

Comment threadstdlib/pathlib.pyi Outdated
Comment threadstdlib/pathlib.pyi
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: ...

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
defsamefile(self, other_path: StrPath) ->bool: ...
defsamefile(self, other_path: StrOrBytesPath|int) ->bool: ...

@AlexWaygoodAlexWaygoodOct 28, 2022

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

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.

@sobolevnsobolevnOct 28, 2022

Copy link
Copy Markdown
MemberAuthor

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:

defsamefile(self, other_path):
st=self.stat()
try:
other_st=other_path.stat()
exceptAttributeError:
other_st=self.__class__(other_path).stat()
returnos.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?

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

defsamefile(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()
exceptAttributeError:
other_st=self._accessor.stat(other_path)
returnos.path.samestat(st, other_st)

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

Comment threadstdlib/pathlib.pyi Outdated
@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

bandersnatch (https://github.com/pypa/bandersnatch)
+ src/bandersnatch_storage_plugins/swift.py: note: In member "write_bytes" of class "SwiftPath":+ src/bandersnatch_storage_plugins/swift.py:408: error: Signature of "write_bytes" incompatible with supertype "Path"+ src/bandersnatch_storage_plugins/swift.py:408: note: Superclass:+ src/bandersnatch_storage_plugins/swift.py:408: note: def write_bytes(self, data: Union[bytes, Union[bytearray, memoryview, array[Any], mmap, _CData, PickleBuffer]]) -> int+ src/bandersnatch_storage_plugins/swift.py:408: note: Subclass:+ src/bandersnatch_storage_plugins/swift.py:408: note: def write_bytes(self, contents: bytes, encoding: Optional[str] = ..., errors: Optional[str] = ...) -> int

@srittau
srittau merged commit d3a87eb into python:masterOct 28, 2022
Sign up for freeto 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.

3 participants

@sobolevn@srittau@AlexWaygood