deeply merge_configs #89
Conversation
src/maison/utils.py
Outdated
| RuntimeError: A dict cannot be merged on top of a non-dict. | ||
| For example, the following would fail: | ||
| `deep_merge({"foo": "bar"}, {"foo": {"baz": "qux"}})` |
There was a problem hiding this comment.
I'm not sure how to fix this. I can't get flake8 to pass no matter what I do.
There was a problem hiding this comment.
looks like these two lines just need two more spaces of indentation?
There was a problem hiding this comment.
That does the trick! Done in 510faa7.
There was a problem hiding this comment.
great! one final hurdle is the coverage which i think could be sorted by adding a test to cover that RuntimeError. sorry i know this ci is quite strict
tests/unit/test_utils.py
Outdated
| param( | ||
| {1: 2, 2: 5}, | ||
| {1: {3: 4}}, | ||
| raises(RuntimeError), | ||
| id="merge-dict-into-scalar", | ||
| ), |
There was a problem hiding this comment.
thanks for adding this extra bit of coverage. how would you feel about moving this to a separate test? i think it would make the tests more readable and would also mean you don't have to import the RaisesContext from the private _pytest api
There was a problem hiding this comment.
Oh, whoops, I misunderstood you.
|
perfect! thanks so much @Jasha10 , great addition. will get a release done shortly |
Implement deep (recursive) merging for nested configs.
Closes #88.