Skip to content

Conversation

@daavoo
Copy link
Contributor

@daavoo daavoo commented Jul 29, 2022

  • Overriding a config value : foo.bar=value
  • Appending a config value : +foo.bar=value
  • Appending or overriding a config value : ++foo.bar=value
  • Removing a config value : ~foo.bar, ~foo.bar=value

See https://hydra.cc/docs/advanced/override_grammar/basic/#modifying-the-config-object


💣 Breaking changes:

  • To modify a list, foo[0]=bar must now be passed as foo.0=bar.

  • Converting a dictionary to a list is not supported by omegaconf.

None of the two were documented.


Closes #4883
Closes #5868
Closes #6129

@daavoo daavoo requested a review from dberenbaum July 29, 2022 08:37
@daavoo daavoo self-assigned this Jul 29, 2022
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To modify a list, foo[0]=bar must now be passed as foo.0=bar.

Comment on lines -118 to -124
Copy link
Contributor Author

@daavoo daavoo Jul 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Converting a dictionary to a list is not supported by omegaconf.

Copy link
Contributor Author

@daavoo daavoo Jul 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All these 4 functions had to be added because we try to preserve comments while modifying the params file.

I don't know if this comment-preserving logic was a user request or how relevant it actually is.
For example, YAML spec says:

Comments are a presentation detail and must not have any effect on the serialization tree or representation graph. 
In particular, comments are not associated with a particular node.

If we discard the idea of trying to preserve comments we could remove quite a lot of code

@dberenbaum

This comment was marked as outdated.

@daavoo

This comment was marked as outdated.

@dberenbaum

This comment was marked as resolved.

@dberenbaum

This comment was marked as resolved.

@dberenbaum

This comment was marked as resolved.

@dberenbaum
Copy link
Contributor

Python support is still pretty buggy but if anything is probably better than before (cc @skshetry)

@daavoo daavoo force-pushed the hydra branch 2 times, most recently from 7e94025 to 1b186d9 Compare July 29, 2022 18:18
@daavoo daavoo force-pushed the hydra branch 2 times, most recently from e988d88 to 90d0457 Compare August 1, 2022 09:45
@daavoo daavoo marked this pull request as ready for review August 1, 2022 09:48
@daavoo daavoo requested a review from a team as a code owner August 1, 2022 09:48
@daavoo daavoo requested a review from pmrowla August 1, 2022 09:48
@daavoo daavoo marked this pull request as draft August 2, 2022 10:27
@daavoo daavoo removed the request for review from pmrowla August 2, 2022 10:27
@daavoo daavoo force-pushed the hydra branch 3 times, most recently from 24b127f to 5aa2943 Compare August 2, 2022 14:43
@daavoo daavoo marked this pull request as ready for review August 2, 2022 14:44
@daavoo daavoo requested a review from pmrowla August 2, 2022 14:44
@daavoo daavoo force-pushed the hydra branch 2 times, most recently from 2db963e to d161ff8 Compare August 4, 2022 10:46
@daavoo daavoo enabled auto-merge (rebase) August 5, 2022 15:06
- Overriding a config value : foo.bar=value
- Appending a config value : +foo.bar=value
- Appending or overriding a config value : ++foo.bar=value
- Removing a config value : ~foo.bar, ~foo.bar=value

See https://hydra.cc/docs/advanced/override_grammar/basic/#modifying-the-config-object

---

Breaking changes:

To modify a list, `foo[0]=bar` must now be passed as `foo.0=bar`.

Modifying a nested list inside a dictionary is not supported by omegaconf.

---

Closes #4883
Closes #5868
Closes #6129
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A: experiments Related to dvc exp feature is a feature

Projects

None yet

3 participants