-
Notifications
You must be signed in to change notification settings - Fork 71
WIP: Patch open, os and os.path builtins #322
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
Conversation
5308425 to
7bf3460
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements experimental patching of Python builtins (open, os, os.path, and glob) to improve third-party library compatibility with CloudPath objects. The changes allow users to override default Python filesystem functions so they work transparently with CloudPath instances.
- Implements comprehensive patching system for
open,osfunctions,os.pathfunctions, andglobmodule - Adds environment variable configuration for automatic patching on import
- Provides context manager support for scoped patching and global patching functions
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
cloudpathlib/patches.py |
Core patching implementation with wrapper functions and context managers |
tests/test_patching.py |
Comprehensive test suite covering all patching functionality |
docs/docs/patching_builtins.ipynb |
Documentation notebook with examples and usage patterns |
docs/docs/script/patching_builtins.py |
Python script version of documentation examples |
docs/mkdocs.yml |
Documentation configuration update to include patching guide |
cloudpathlib/__init__.py |
Module exports and environment variable handling for automatic patching |
cloudpathlib/cloudpath.py |
Support for variadic constructor arguments and improved error handling |
cloudpathlib/client.py |
CloudPath factory method updated to support variadic arguments |
cloudpathlib/exceptions.py |
New exception types for patching and file operations |
cloudpathlib/http/httppath.py |
Constructor updated to support variadic arguments |
cloudpathlib/http/httpclient.py |
Added _is_file_or_dir method implementation |
cloudpathlib/local/localclient.py |
Added _is_file_or_dir method implementation |
Makefile |
Debug test configuration update |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
|
@jayqi ok, I think that this is finally ready to ship if test suite passes! Let me know if you want to do a review—it's pretty beefy so could do synchronous if easier. |
pjbull
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few tweaks before shipping
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #322 +/- ##
========================================
- Coverage 93.8% 93.8% -0.1%
========================================
Files 26 27 +1
Lines 1949 2164 +215
========================================
+ Hits 1830 2030 +200
- Misses 119 134 +15
🚀 New features to boost your workflow:
|
Right now, libs that use
opendirectly,fspathor just assume paths are strings and can be manipulated don't work withCloudPathobjects. (Related issues: #315, #73, #128)Totally WIP and not sure that we want to introduce and support this, but it may increase compatibility significantly to have the option at least for users to patch the builtin functions.
The biggest limitations are:
path = dir + os.sep + file)Other potential todos for this PR:
globmodule as wellpandashas all kinds of formats,rasteriomay be interesting,xarrayhas come up,Pillowmay be a common use case)osfunctionsos.fspathto allow returningCloudPathopen = patch_open()sinceopenexists as alocalin the notebook namespaceCurious for your thoughts @jayqi !
Experimental instructions
pip install git+https://github.com/drivendataorg/cloudpathlib@patch-builtinsCLOUDPATHLIB_PATCH_OPEN=Truebefore any import call tocloudpathlibopenor third-party libraries that use open.