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.

Sign up for GitHub

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

Jump to bottom

--cfg=hydra for read-only top-level config #1542

Merged
merged 2 commits into from
Apr 14, 2021
Merged

--cfg=hydra for read-only top-level config #1542

merged 2 commits into from
Apr 14, 2021

Conversation

jieru-hu
Copy link
Contributor

@jieru-hu jieru-hu commented Apr 6, 2021

Closes #1461

@jieru-hu jieru-hu requested a review from omry April 6, 2021 21:29
@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 Apr 6, 2021
omry
omry reviewed Apr 7, 2021
View reviewed changes
Copy link
Collaborator

@omry omry left a comment

Choose a reason for hiding this comment

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

I commented on the original issue with a few related problems, can you check that those are covered as well?

hydra/_internal/hydra.py Outdated Show resolved Hide resolved
tests/test_hydra.py Outdated Show resolved Hide resolved
tests/test_hydra.py Outdated Show resolved Hide resolved
@jieru-hu
Copy link
Contributor Author

jieru-hu commented Apr 9, 2021

I commented on the original issue with a few related problems, can you check that those are covered as well?

sorry missed the comment. added fix and test for help and info.

This would a bit more compact:

nice!

To make this more compact, you can get a specific value: -p baud_rate

fixed!

@jieru-hu jieru-hu requested a review from omry April 12, 2021 18:17
omry
omry approved these changes Apr 14, 2021
View reviewed changes
tests/test_hydra.py
Comment on lines +1241 to +1245
(["--cfg", "job", "-p", "baud_rate"], "19200"),
(["--cfg", "hydra", "-p", "hydra.job.name"], "frozen"),
(["--info", "config"], "baud_rate: 19200"),
(["--hydra-help"], "== Flags =="),
(["--help"], "frozen is powered by Hydra."),
Copy link
Collaborator

Choose a reason for hiding this comment

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

neat :)

@jieru-hu jieru-hu merged commit ce6add4 into facebookresearch:master Apr 14, 2021
@jieru-hu jieru-hu deleted the cli branch July 20, 2021 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@omry omry omry approved these changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug] --cfg=hydra CLI fails if top-level config is read-only
3 participants
@jieru-hu @omry @facebook-github-bot