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

Maybe change _file to update existing dictionary rather than replacing it? #1

Open
fferflo opened this issue Jun 29, 2023 · 1 comment

Comments

@fferflo
Copy link

fferflo commented Jun 29, 2023

Hey, I love this library!

As far as I understand, using _file removes all existing keys at the same tree level and replaces them with the content of the file, like so:

# conf/base.yaml
sub:
  a: 1 # Is removed when _file is loaded
  _file: update

# conf/sub/update.yaml
b: 2

# Result:
sub:
  b: 2

What do you think about updating the existing dictionary with the file values instead? The result would then be:

# Result:
sub:
  a: 1
  b: 2

This would keep existing keys and would essentially work like default values for the loaded file, so that common values do not have to repeated in every file I could import.

The tree would ideally be updated recursively, so something like this might work:

+ def _update_recursively(node, key, update):
+     if isinstance(node[key], dict) and isinstance(update, dict):
+         for k, v in update.items():
+             if k in node[key]:
+                 _update_recursively(node[key], k, v)
+             else:
+                 node[key][k] = v
+     else:
+         node[key] = update

...

In https://github.com/mattiasarro/confr/blob/main/src/confr/models.py#L119
- conf_dict[k] = read_yaml(conf_fp, verbose=verbose)
+ _update_recursively(conf_dict, k, read_yaml(conf_fp, verbose=verbose))
+ del conf_dict[k]["_file"]
@mattiasarro
Copy link
Owner

Hey @fferflo, thanks for the feedback! Sorry I didn't see it before, I'm not used to checking my github notifications.

I am open to making this change - that is, if someone (you) is still interested in the change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants