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

hydra: Use OmegaConf.to_yaml for dumping .yaml output. #8587

Merged
merged 1 commit into from
Nov 19, 2022
Merged

Conversation

daavoo
Copy link
Contributor

@daavoo daavoo commented Nov 18, 2022

Combining OmegaConf.to_object with our yaml DUMPER (ruaml.yaml) was not correctly handling strings like 'no'.

OmegaConf.to_yaml uses a custom string representer that correctly handles those cases.

Fixes #8583

@daavoo daavoo self-assigned this Nov 18, 2022
@daavoo daavoo added the bugfix fixes bug label Nov 18, 2022
@codecov
Copy link

codecov bot commented Nov 18, 2022

Codecov Report

Base: 94.28% // Head: 94.28% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (f91a548) compared to base (dae500d).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8587      +/-   ##
==========================================
- Coverage   94.28%   94.28%   -0.01%     
==========================================
  Files         431      431              
  Lines       32919    32931      +12     
  Branches     4605     4607       +2     
==========================================
+ Hits        31039    31048       +9     
- Misses       1458     1461       +3     
  Partials      422      422              
Impacted Files Coverage Δ
dvc/utils/hydra.py 81.53% <100.00%> (+0.89%) ⬆️
tests/func/utils/test_hydra.py 100.00% <100.00%> (ø)
tests/unit/repo/experiments/conftest.py 88.67% <0.00%> (-5.67%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@daavoo daavoo requested a review from dberenbaum November 18, 2022 18:40
dvc/utils/hydra.py Outdated Show resolved Hide resolved
@daavoo daavoo enabled auto-merge (squash) November 19, 2022 09:28
Combining `OmegaConf.to_object` with our yaml DUMPER (ruaml.yaml) was not correctly handling strings like 'no'.

`OmegaConf.to_yaml` uses a custom string representer that correctly handles those cases.

Fixes #8583

Co-authored-by: Dave Berenbaum <dave@iterative.ai>
@daavoo daavoo merged commit b0a9241 into main Nov 19, 2022
@daavoo daavoo deleted the fix-8583 branch November 19, 2022 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix fixes bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

exp run: Incorrect string handling
2 participants