8000 Change return type of PureS3Path class methods to Self by alexjbuck · Pull Request #199 · liormizr/s3path · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Change return type of PureS3Path class methods to Self #199

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

Jump to bottom
Merged
merged 1 commit into from
Jun 14, 2025
Merged

Change return type of PureS3Path class methods to Self #199

merged 1 commit into from
Jun 14, 2025

Conversation

alexjbuck
Copy link
Contributor
@alexjbuck alexjbuck commented Jun 6, 2025 •
edited
Loading

Thanks for adding type hints to s3path!

I saw your comments about type hints adding to the maintenance burden, and I don't want that to be the case because the benefit of type hints are great. This is a PR to address what I believe is an error in the type hint implementation added in 0.6.3.

Closes #198

This PR changes the return type of PureS3Path.from_uri and PureS3Path.from_bucket_key to be Self instead of PureS3Path. Both class methods return an instance of cls which is not necessarily a PureS3Path if the class method is invoked from a subclass.

# 0.6.3 type behavior
>> S3Path.from_uri('s3://bucket/key')
<< PureS3Path('/bucket/key')

>> PureS3Path.from_uri('s3://bucket/key')
<< PureS3Path('/bucket/key')

# Proposed type behavior
>> S3Path.from_uri('s3://bucket/key')
<< S3Path('/bucket/key')

>> PureS3Path.from_uri('s3://bucket/key')
<< PureS3Path('/bucket/key')

I believe this is the desired behavior because this allows construction of a concrete S3Path from a uri string.

The alternative is that PureS3Path.from_uri should only ever return a PureS3Path and not be allowed to return an S3Path or any other subclass of PureS3Path. If this is the case, instead of changing the return type hints, the implementation of the class methods should be changed to return a PureS3Path instead of cls.

@liormizr liormizr merged commit 222e01e into liormizr:master Jun 14, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liormizr liormizr liormizr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

S3Path.from_uri returns a PureS3Path instead of S3Path
2 participants
@alexjbuck @liormizr
0