Skip to content
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.

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

add new pathlib base classes for 3.13 #12937

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

tungol
Copy link
Contributor

@tungol tungol commented Oct 31, 2024

3.13 adds these new (private for now) base classes for pathlib which are not actually ABCs despite the module name.

@tungol tungol marked this pull request as draft October 31, 2024 22:26

This comment has been minimized.

@tungol tungol marked this pull request as ready for review October 31, 2024 22:41
@tungol
Copy link
Contributor Author

tungol commented Oct 31, 2024

Is there a way to make mypy-primer run for 3.13 for this MR? If there's anything off, that's where it'd show up.

This comment has been minimized.

def is_socket(self) -> bool: ...
def samefile(self, other_path: StrPath) -> bool: ...

# Adapted from builtins.open
Copy link
Member

Choose a reason for hiding this comment

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

I don't know if we should require all implementations of this ABC to duplicate this stack of overloads. Not sure what the alternative would be, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hadn't considered that aspect when copying it over. I think the alternative is to define only the most generic form of open on the base class, and implementations can have more specific overloads as needed. I'll update the MR for that.

Copy link
Contributor Author

@tungol tungol Nov 2, 2024

Choose a reason for hiding this comment

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

stubtest didn't like that... I thought it would work based on a simplified test I did locally, but possibly I misunderstood something. I'll need to look at it later, so I'll put this in draft for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also the same concern possibly applies to the overloads on ParserBase. I copied the definitions in the MR right now for that class from the versions that exist in posixpath.pyi.

@tungol tungol marked this pull request as draft November 2, 2024 21:05

This comment has been minimized.

@tungol
Copy link
Contributor Author

tungol commented Nov 2, 2024

Adding the default value to the final overload makes the pipeline pass, and shouldn't affect anything overall due to mypy's "pick the first match" handling of overloads.

@tungol tungol marked this pull request as ready for review November 2, 2024 23:50
Copy link
Contributor

github-actions bot commented Nov 2, 2024

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

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.

3 participants