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

fix: uproot was exposed in one place to dask's _task_spec overhaul #1352

Merged
merged 3 commits into from
Dec 16, 2024

Conversation

lgray
Copy link
Contributor

@lgray lgray commented Dec 16, 2024

c.f. dask-contrib/dask-awkward#556

There don't appear to be any other places where uproot directly accesses dask innards.

@lgray lgray changed the title fix: uproot was exposed in one plat to dask's _task_spec overhaul fix: uproot was exposed in one place to dask's _task_spec overhaul Dec 16, 2024
@lgray
Copy link
Contributor Author

lgray commented Dec 16, 2024

@jpivarski please merge this as soon as it's all passing, we will need a timely release as well.

@lgray
Copy link
Contributor Author

lgray commented Dec 16, 2024

ah - of course this waits on the dask-awkward release first :-p

@pfackeldey
Copy link
Collaborator

pfackeldey commented Dec 16, 2024

@lgray there's no reason to increase the dask-awkward version requirement, or is there? The change checks for an update in dask for the new task_spec, but still handles the old case. (I think this PR is independent of the dask-awkward PR aswell in this sense)

edit: oh well, ok, there is a need for this, because without there could be a state where dask uses new task_spec, uproot can handle it (with this PR), but dask-awkward doesn't. Nevermind then, sorry for the noise!

@lgray
Copy link
Contributor Author

lgray commented Dec 16, 2024

And it's only for the dev extra as well, and since we check back to py39 we test all code paths.

Copy link
Member

@jpivarski jpivarski left a comment

Choose a reason for hiding this comment

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

I see that this follows upstream changes in Dask and has Uproot taking a new minimum dask-awkward version.

Looks good to me! I'll make a new release right after merging this PR.

@jpivarski jpivarski merged commit 2ba58f2 into scikit-hep:main Dec 16, 2024
26 checks passed
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