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

[ENH] should set_config and get_config be able to address deep configs? #282

Open
fkiraly opened this issue Jan 31, 2024 · 0 comments
Open
Labels
API design API design & software architecture enhancement Adding new functionality

Comments

@fkiraly
Copy link
Contributor

fkiraly commented Jan 31, 2024

here's a question: should there be a deep arg to get_config, like get_params? That way, we could avoid changes to _clone, as then we just need to add the deep arg in clone, once. It would be "cleaner" imo since it completely decouples the param logic from the config logic (coupling/cohesion improvement).

That is, should get_config, set_config of the top node object support getting/setting of component node configs - like get_params, set_params - or do you have to address the components directly.

I.e., compare

# how it would work right now
composite = Composite(comp1=Component1(), comp2=Component2().set_config(foo="bar"))
# vs how it could also work
composite = Composite(comp1=Component1(), comp2=Component2())
composite.set_config(comp2__foo="bar")

Both ways are already making the assumption that clone should keep configs across all nodes.

Originally posted by @fkiraly in #276 (comment), moved here for discussion.

@fkiraly fkiraly added API design API design & software architecture enhancement Adding new functionality labels Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API design API design & software architecture enhancement Adding new functionality
Projects
None yet
Development

No branches or pull requests

1 participant