cloudpathlib
cloudpathlib copied to clipboard
Anypath abstractmethods
Disclaimer: I quickly hacked this as a draft/suggestion. There might be better/nicer methods to achieve the same.
I suggest to add some abstractmethods to AnyPath so that the methods available for both CloudPath and pathlib.Path can be seen when AnyPath is used as type annotation. It would be nice if there are more elegant solutions but I think the overall behavior would be useful.
Currently missing:
- Docstrings for functions that differ between pathlib.Path and CloudPath
- CloudPathMeta could inherit from AnyPathMeta to avoid code duplication
I probably won't be around the next days but wanted to leave this as starting point to discuss.
Codecov Report
Merging #266 (3d455b3) into master (98ed2e5) will decrease coverage by
2.4%
. The diff coverage is73.2%
.
@@ Coverage Diff @@
## master #266 +/- ##
========================================
- Coverage 94.8% 92.3% -2.5%
========================================
Files 21 21
Lines 1321 1453 +132
========================================
+ Hits 1253 1342 +89
- Misses 68 111 +43
Impacted Files | Coverage Ξ | |
---|---|---|
cloudpathlib/anypath.py | 76.1% <71.8%> (-23.9%) |
:arrow_down: |
cloudpathlib/cloudpath.py | 93.5% <100.0%> (ΓΈ) |
|
cloudpathlib/s3/s3client.py | 92.9% <0.0%> (-2.7%) |
:arrow_down: |
cloudpathlib/gs/gsclient.py | 92.8% <0.0%> (-1.8%) |
:arrow_down: |
Any opinion on this @pjbull or @jayqi? I'm happy to make this a polished PR or go after another solution if you have any suggestions.
Sorry @chbehrens. Delay is because, as you point out, the current implementation is a lot of boilerplate which seems less than ideal just to support use of AnyPath
as a type hint (does typing things as Union[CloudPath, Path]
instead work for your purposes?). I don't have a better implementation off the top of my head, and like you say we should think about it in the context of our overall class hierarchy. Happy to open an issue for discussion, but I'm not super keen on this PR as is. Maybe @jayqi has a different opinion.
Don't get me wrong, I'm also not keen on it the way it is. I just did it to show what I'd like to achieve in the end behavior-wise but preferably via something more meta programming like. That's why I made it a draft and was wondering whether you have some ideas or what you think of the overall intended behavior. I'm sorry if it's the wrong format here, I just thought a draft PR might demonstrate it better than an issue.
@ringohoffman Do you have thoughts on this after implementing your changes in #298? Is there a cleaner/easier way to get useful type hinting for AnyPath
without duplicating all the signatures?
I was asking about this over at https://github.com/microsoft/pyright/issues/4331 and got back a few suggestions... and the virtual superclass approach was not one of them. I will have to play around to see which results in the most expected / convenient typing experience.
Much appreciated! Curious what you discover.
It does some interesting stuff in regard to typing.
π π π
@chbehrens I'm going through old PRs to keep them moving.
I think that I've come around on this. I was hoping for some cleaner upstream fix that makes this kind of polymorphic class work, but I don't think there's another way to get useful completions for AnyPath
until something like #347 comes in upstream in Python (and we can subclass from that).
Would you be willing to rebase this on the latest and add tests/docs?
Outside of that, I am curious if one of the other ideas in this thread might work: https://github.com/microsoft/pyright/issues/4331#issuecomment-1350209969
It does seem that Protocol
is potentially a friendlier implementation than ABC
since it is slightly less boilerplate and plays nicer with static type checkers, but I'm not sure it solves all our issues without some experimentation.
I'll try to have a look at it if I find time but let's just close this, an issue would be better suited to track the topic if necessary.
I've experimented with Protocols, and one of the main problems IIRC is that the Pydantic validation methods were getting caught up in it. There's also some annoying stuff with API changes to Path
over different Python versions, and not being able to match all of them.