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 tiny test for ToPickle #6021

Merged
merged 2 commits into from
Mar 30, 2022
Merged

Add tiny test for ToPickle #6021

merged 2 commits into from
Mar 30, 2022

Conversation

mrocklin
Copy link
Member

  • Closes #xxxx
  • Tests added / passed
  • Passes pre-commit run --all-files

@github-actions
Copy link
Contributor

github-actions bot commented Mar 30, 2022

Unit Test Results

       17 files  +       6         17 suites  +6   9h 5m 22s ⏱️ + 3h 24m 26s
  2 703 tests +     28    2 621 ✔️ +     30       81 💤  -     2  1 ±0 
22 967 runs  +8 476  21 740 ✔️ +8 024  1 226 💤 +452  1 ±0 

For more details on these failures, see this check.

Results for commit 2ac24e5. ± Comparison against base commit ed48736.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@madsbk madsbk left a comment

Choose a reason for hiding this comment

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

LGTM but I suggest that the test is moved to test_to_pickle.py.

@mrocklin
Copy link
Member Author

Planning to merge when CI finishes.

@mrocklin mrocklin merged commit c8a97a3 into dask:main Mar 30, 2022
@mrocklin mrocklin deleted the test-ToPickle branch March 30, 2022 19:31
@mrocklin
Copy link
Member Author

Also, it looks like ToPickle fails with inproc comms.

@mrocklin
Copy link
Member Author

Maybe we need this?

diff --git a/distributed/protocol/serialize.py b/distributed/protocol/serialize.py
index b4daf5bd..48091cc5 100644
--- a/distributed/protocol/serialize.py
+++ b/distributed/protocol/serialize.py
@@ -611,8 +611,12 @@ def nested_deserialize(x):
                     x[k] = replace_inner(v)
                 elif typ is Serialize:
                     x[k] = v.data
+                elif typ is ToPickle:
+                    x[k] = v.data
                 elif typ is Serialized:
                     x[k] = deserialize(v.header, v.frames)
+                elif typ is Pickled:
+                    x[k] = pickle.loads(v.header, buffers=v.frames)
 
         elif type(x) is list:
             x = list(x)
@@ -622,8 +626,12 @@ def nested_deserialize(x):
                     x[k] = replace_inner(v)
                 elif typ is Serialize:
                     x[k] = v.data
+                elif typ is ToPickle:
+                    x[k] = v.data
                 elif typ is Serialized:
                     x[k] = deserialize(v.header, v.frames)
+                elif typ is Pickled:
+                    x[k] = pickle.loads(v.header, buffers=v.frames)
 
         return x
 

mrocklin added a commit to mrocklin/distributed that referenced this pull request Mar 31, 2022
@madsbk
Copy link
Contributor

madsbk commented Apr 1, 2022

#6044 should fix the nproc issue.

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.

2 participants