-
Notifications
You must be signed in to change notification settings - Fork 121
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
update dask vine executor to new dask graphs #4015
update dask vine executor to new dask graphs #4015
Conversation
Figuring out an error with coffea's CI. |
Ok, it seems to be working. |
I am going to try with the new one. |
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.
The overall approach makes sense to me.
I am concerned about how we deal with backwards/forwards compatibility, esp for physics applications which have not necessarily been ported to the new version of Dask.
Do you have any ideas how to support this migration? We don't need to keep compatibility forever, but we also can't expect our key users to all change at the same time.
At the very least, we need to be very explicit in the manual about the supported version, and also note that in our Conda environments.
* DaskVine to new graph representation * update simple graph example * fix bug with depth * always convert from legacy representation, for now * check for dask in test * do not import DaskVineDag if dask not available * update function calls * lint * handle generic container graph nodes * add warning about dask version * example_to_revert * remove print statement
make test
Run local tests prior to pushing.make format
Format source code to comply with lint policies. Note that some lint errors can only be resolved manually (e.g., Python)make lint
Run lint on source code prior to pushing.