Skip to content

Conversation

@milesgranger
Copy link
Collaborator

@milesgranger milesgranger commented Jan 10, 2024

Will close dask/dask#10760

TL/DR; dask-expr needs an extra check for a lambda function in _normalize_lambda since free functions are also instances of types.LambdaType (it's actually an alias to types.FunctionType...because, why not?). And failing on Python 3.12 because that's the only one that also includes a dask-expr dependency.


git bisect'd to #10676 (the last tests in that PR unsurprisingly also show the failing 3.12 test)

Running the failing test by itself (test_use_cloudpickle_to_tokenize_functions_in__main__) will work fine, but running the module (python -m pytest -k test_base) will bring the issue out. Then commenting out the two tests added in that PR will also allow everything to pass.

Specifically, running by itself returns the expected bytes but running the module the normalize_token will instead return a string, like '<function inc at 0x7f5e97dad9e0>', giving the error we saw in the failing test.

TypeError: 'in ' requires string as left operand, not bytes

This was because dask-expr's _utils._normalize_lambda registered on types.LambdaType. Adding an extra check for the 'lambda' function name as well as <locals> to avoid cloudpipe/cloudpickle#385 resolves the issue. Just so happens the tests added in the referenced PR managed to import dask-expr and thus added _normalize_lambda to the mix.

@milesgranger milesgranger force-pushed the milesgranger/fix-dask-py12-ci branch from 36cba4a to 6ac3bb9 Compare January 10, 2024 08:56
@milesgranger milesgranger force-pushed the milesgranger/fix-dask-py12-ci branch from 6ac3bb9 to 98c6684 Compare January 10, 2024 09:04
@milesgranger milesgranger requested a review from phofl January 10, 2024 09:08
@milesgranger milesgranger force-pushed the milesgranger/fix-dask-py12-ci branch from 2b25a2c to 8519d56 Compare January 10, 2024 09:11
Copy link
Collaborator

@phofl phofl left a comment

Choose a reason for hiding this comment

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

great find!

@phofl phofl merged commit 042ddb8 into dask:main Jan 10, 2024
@phofl
Copy link
Collaborator

phofl commented Jan 10, 2024

thx

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.12 CI started failing a couple of days ago

2 participants