-
Notifications
You must be signed in to change notification settings - Fork 27
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
Fix for dask 2024.12 #156
base: master
Are you sure you want to change the base?
Fix for dask 2024.12 #156
Conversation
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.
Thanks for the PR. One small request.
|
||
from rechunker.types import ParallelPipelines, Pipeline, PipelineExecutor | ||
|
||
# Change in how dask collection token are given to blockwise() |
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.
As of dask/dask#11675, there's a public dask.task_spec
we can import this from. I think that was released in 2025.1.0.
Could you try adding another version check and importing it from the public location if possible?
This fixes #155, allowing
rechunker
to be used with dask >= 2024.12.0.The change in dask's
blockwise
was introduced in dask/dask#11568 and requires keys pointing to other dask collections to be wrapped withinTaskRef
objects. The latter were introduced in dask 2024.09, so to preserve retro-compatibility (rechunker
doesn't pindask
), I added a if-else that checks dask's version.I tested against dask 2024.9, 2024.11.2, 2024.12 and 2025.2.
Installing
apache-beam
requiresdask < 2024.8.0
because the former needscloudpickle < 2.3
and the latter>=3
, thus I didn't test if my change had impacts on theapache-beam
tests. (I guess it shouldn't...)