-
Notifications
You must be signed in to change notification settings - Fork 64
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
allow for roundtrips of cloudpaths through pickle serialization #454
allow for roundtrips of cloudpaths through pickle serialization #454
Conversation
from cloudpathlib import CloudPath | ||
|
||
|
||
def test_pickle_roundtrip(): |
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.
If there is an existing file this would be better suited to put it within let me know and happy to move it!
I verified that this does repro the issue without the above change.
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.
Separate file for serializing is fine. Can you also move this test over to the new file so they are together?
cloudpathlib/tests/test_cloudpath_file_io.py
Lines 457 to 472 in 08b018b
def test_pickle(rig, tmpdir): | |
p = rig.create_cloud_path("dir_0/file0_0.txt") | |
with (tmpdir / "test.pkl").open("wb") as f: | |
pickle.dump(p, f) | |
with (tmpdir / "test.pkl").open("rb") as f: | |
pickled = pickle.load(f) | |
# test a call to the network | |
assert pickled.exists() | |
# check we unpickled, and that client is the default client | |
assert str(pickled) == str(p) | |
assert pickled.client == p.client | |
assert rig.client_class._default_client == pickled.client |
This avoids an exception thrown because the _client is not serialized into the pickled object, and thus when __getstate__ is called the second time, there is no _client field to delete. Closes drivendataorg#450
47490cf
to
514d447
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 454-local #454 +/- ##
===========================================
- Coverage 93.7% 93.4% -0.4%
===========================================
Files 23 23
Lines 1654 1655 +1
===========================================
- Hits 1551 1546 -5
- Misses 103 109 +6
|
from cloudpathlib import CloudPath | ||
|
||
|
||
def test_pickle_roundtrip(): |
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.
Separate file for serializing is fine. Can you also move this test over to the new file so they are together?
cloudpathlib/tests/test_cloudpath_file_io.py
Lines 457 to 472 in 08b018b
def test_pickle(rig, tmpdir): | |
p = rig.create_cloud_path("dir_0/file0_0.txt") | |
with (tmpdir / "test.pkl").open("wb") as f: | |
pickle.dump(p, f) | |
with (tmpdir / "test.pkl").open("rb") as f: | |
pickled = pickle.load(f) | |
# test a call to the network | |
assert pickled.exists() | |
# check we unpickled, and that client is the default client | |
assert str(pickled) == str(p) | |
assert pickled.client == p.client | |
assert rig.client_class._default_client == pickled.client |
Looks good, thanks! One small ask to move the other pickle test to the same place |
@kujenga Planning a release soon—will you have a chance to wrap this PR? |
* allow for roundtrips of cloudpaths through pickle serialization (#454) This avoids an exception thrown because the _client is not serialized into the pickled object, and thus when __getstate__ is called the second time, there is no _client field to delete. Closes #450 * pickle tests --------- Co-authored-by: Aaron Taylor <[email protected]>
Thank you very much! Apologies I missed the earlier notification |
This avoids an exception thrown because the client is not serialized into the pickeld object, and thus when getstate is called the second time, there is no _client field to delete.
Closes #450
Contributor checklist:
CONTRIBUTING.md
Closes #issue
appears in the PR summary (e.g.,Closes #123
).HISTORY.md
with the issue that is addressed and the PR you are submitting. If the top section is not `## UNRELEASED``, then you need to add a new section to the top of the document for your change.