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

Instantiate refactor + support for instantiating dataclasses #1418

Merged
merged 18 commits into from
Feb 24, 2021

Conversation

omry
Copy link
Collaborator

@omry omry commented Feb 23, 2021

Taking over #1236 (cc @guillaumegenthial).

Closes #1183
Closes #1171
Closes #1410

Changes:

  1. removed parse mode for now (It was not documented and not tested and it didn't seem to be needed for anything).
  2. Refactor tests to allow testing multiple instantiate functions in parallel
  3. Added a second instantiate function and simplified it (187 vs 240 lines).
  4. I also added some more tests covering convert modes.
  5. After taking a close look, I agree that it's very difficult to support passthrough of dataclass instances so for now I am officially unsupporting it. docs says that passthrough dataclasses or instances are interpreted as configs.
  6. removed the implementation by @guillaumegenthial in favor of the cleaner implementation.
  7. Interface changes: removed recursive and convert from the signature of instantiate. Removed support for instantiating a ListConfig (Top level config must be a DictConfig, plain dict or dataclass or instance compatible with OmegaConf.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 23, 2021
@omry omry changed the title Instantiate refactor Instantiate refactor + support for instantiating dataclasses Feb 24, 2021
@omry omry merged commit 1df6e29 into master Feb 24, 2021
@omry omry deleted the instantiate-refactor branch February 24, 2021 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
3 participants