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

changes from_kwargs to from_config #954

Merged
merged 1 commit into from
Oct 18, 2023
Merged

changes from_kwargs to from_config #954

merged 1 commit into from
Oct 18, 2023

Conversation

normanrz
Copy link
Member

While trying out the dask executor in voxelytics, I noticed that the signature for creating an executor is not consistent. That makes this breaking change necessary. I don't think it is a big deal, because I doubt anybody else has used it in the meantime.

@normanrz normanrz requested a review from philippotto October 18, 2023 14:47
@normanrz normanrz self-assigned this Oct 18, 2023
@philippotto
Copy link
Member

While trying out the dask executor in voxelytics, I noticed that the signature for creating an executor is not consistent.

Can you link where else from_config is defined? I don't find anything like that in our repo or in the python documentation for executors? 🤔 I'm probably missing the forest for the trees.

@normanrz
Copy link
Member Author

The difference is that previously the **kwargs were directly passed to the Client constructor. Now, the job_resources part of the kwargs is used (by not using **)

@normanrz normanrz enabled auto-merge (squash) October 18, 2023 15:06
@normanrz normanrz merged commit 5529367 into master Oct 18, 2023
29 checks passed
@normanrz normanrz deleted the dask-change branch October 18, 2023 17:21
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.

2 participants