Fix bug with merging configuration files #65
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There is a function in
collections.py
that merges two dictionaries, and is used to load a user configuration into an existing default configuration.The basic logic is that it iterates over each key of the user config, and if that key has a non-dict value, it updates that key in the default config with the user value. However, if the key is a dict, then it recurses into that dict and performs a nested merge. The idea is that this allows merging of nested dictionaries without forcing the user dict to have every key in the default dict.
However, the merge function only recursed if
isinstance(config, dict)
was True... andConfigs
are not, in fact,dicts
. This resulted in very bad merges whenever a user config was present that was at all different from the default config, since the merge function wouldn't treat theconfig
as adict
and do a nested merge.This PR adjusts the merge function to test for
isinstance(config, MutableMapping)
, adds merge unit tests, and also adds a unit test to make sure user configs are loaded appropriately.