Skip to content

fix: make dask_property picklable - #629

Merged
martindurant merged 1 commit into
dask-contrib:mainfrom
ikrommyd:fix/picklable-dask-property
Jul 22, 2026
Merged

fix: make dask_property picklable#629
martindurant merged 1 commit into
dask-contrib:mainfrom
ikrommyd:fix/picklable-dask-property

Conversation

@ikrommyd

@ikrommyd ikrommyd commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

🤖 AI text below 🤖

A class using @dask_property cannot be pickled by value:

import cloudpickle
import dask_awkward as dak

class Thing:
    @dak.dask_property
    def foo(self):
        return 1

    @foo.dask
    def foo(self, array):
        return 2

cloudpickle.dumps(Thing)
# TypeError: cannot pickle '_DaskProperty' object

property keeps fget/fset/fdel in C-level slots and provides no reduction of its own, so CPython refuses outright — pickle.dumps(property(f)) raises the same error. _DaskProperty inherits that refusal even though it does have an instance __dict__.

This matters because cloudpickle only pickles a class by reference when it is reachable as getattr(module, cls.__qualname__). Anything else — a behavior mixin written in __main__ or in a notebook cell, or one whose generated *Record/*Array classes have been shadowed in their module — gets pickled by value, which walks the class dict and hits the property. The result is that such behaviors cannot be sent to a distributed worker at all.

_DaskProperty now defines __reduce__, round-tripping fget/fset/fdel/__doc__/_dask_get.

_DaskMethod needs no change: it is a plain Python class whose whole state lives in the instance __dict__, so the default reduction already handles it and cloudpickle serializes the contained function and closure by value. It gets a test to keep it that way.

Found while debugging the coffea side of this, where the same copy of _DaskProperty broke FCCSchema under a distributed client: scikit-hep/coffea#1603.

@codecov-commenter

codecov-commenter commented Jul 21, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.03%. Comparing base (3d90b72) to head (6c962dd).
⚠️ Report is 33 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #629      +/-   ##
==========================================
- Coverage   90.05%   90.03%   -0.03%     
==========================================
  Files          24       24              
  Lines        3610     3601       -9     
==========================================
- Hits         3251     3242       -9     
  Misses        359      359              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

property keeps fget/fset/fdel in C slots and has no reduction of its own, so
a behavior class defined in __main__ or a notebook could not be sent to a
distributed worker. _DaskMethod already pickled fine; it just gets a test.

Assisted-by: ClaudeCode:claude-opus-4.8
@ikrommyd
ikrommyd force-pushed the fix/picklable-dask-property branch from 9578bb4 to 6c962dd Compare July 21, 2026 19:16
@ikrommyd
ikrommyd requested a review from martindurant July 21, 2026 19:38
@martindurant

Copy link
Copy Markdown
Collaborator

OK. Looks fine. Wonder how you came onto this? Wonder whether the classes that have the property should be changed to have their own reduce() - but there's no downside to this either.

@martindurant
martindurant merged commit d3da5eb into dask-contrib:main Jul 22, 2026
27 checks passed
@ikrommyd

Copy link
Copy Markdown
Collaborator Author

OK. Looks fine. Wonder how you came onto this? Wonder whether the classes that have the property should be changed to have their own reduce() - but there's no downside to this either.

Claude found it while working on scikit-hep/coffea#1603. In coffea to make dask-awkward optional dependency we just vendored dask method and dask property. So I thought I'd just upstream the patch here too.

@ikrommyd
ikrommyd deleted the fix/picklable-dask-property branch July 22, 2026 18:27
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.

3 participants