Skip to content

[WIP] Fix dask.config dict usage in favor of config object interface#2442

Open
djhoese wants to merge 1 commit into
dask:mainfrom
djhoese:refactor-config-object
Open

[WIP] Fix dask.config dict usage in favor of config object interface#2442
djhoese wants to merge 1 commit into
dask:mainfrom
djhoese:refactor-config-object

Conversation

@djhoese

@djhoese djhoese commented Dec 27, 2018

Copy link
Copy Markdown

This PR goes along with dask/dask#4328 (or its predecessor dask/dask#4099).

It was discussed in related issues that it would be best if dask used a configuration object instead of the global config module in dask/config.py. I started doing this in 4099 and then ended up creating a separate config package named donfig which is what 4328 uses. The only failing tests remaining for me locally were related to the distributed package and its use of dask/config.py:config; the internal dictionary representing the configuration.

This PR shows one way of working around the transition from internal dict to public config object. I couldn't really think of a better solution. New versions of donfig support the x in config usage, but old dask config doesn't so it just makes the code ugly.

TODO: Need to fix up tests and find the easiest way to present these changes for the dask devs.

@djhoese

djhoese commented Dec 28, 2018

Copy link
Copy Markdown
Author

@mrocklin What is your (and other dask devs) preferred way of handling circular dependency releases between dask packages? For example, dask/dask#4328 is failing because distributed expects to use the dask.config object as a module. But fixing distributed to use dask's config object like a class is failing in this PR because the tests need to use dask's config object that hasn't been merged yet.

I could modify donfig (or a dask config object implementation) to have more backwards compatible interfaces for things I think, but I'm not sure I can handle all cases. Even if dask doesn't want to add donfig as a dependency I think it should at least have a config object rather than a config module.

Thoughts?

Base automatically changed from master to main March 8, 2021 19:03
@djhoese djhoese requested a review from fjetter as a code owner January 23, 2024 10:57
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.

1 participant