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

Assignments should strip off any ConfigNode wrappers. #15

Open
wimglenn opened this issue Dec 8, 2022 · 2 comments
Open

Assignments should strip off any ConfigNode wrappers. #15

wimglenn opened this issue Dec 8, 2022 · 2 comments

Comments

@wimglenn
Copy link
Contributor

wimglenn commented Dec 8, 2022

I noticed that Config.__eq__ isn't implemented so we just get object.__eq__ comparison (identity based)

>>> from configurator import Config
>>> c1 = Config({"k": "v"})
>>> c2 = Config({"k": "v"})
>>> c1 == c2
False

Would it be possible to get a content-based equality comparison? In the simple example above we could just use c1.data == c2.data, but my real use-case has a nested configuration so comparing data attributes doesn't work either: the nested values are instances of ConfigNode, which also has identity-based comparison.

My use-case is in testing comparisons, where we want to assert some configs from multiple sources have been parsed and layered the way we expected, without having to recursively extract all the content from the Config instance. i.e., just compare with an "expected" Config or ConfigNode instance directly.

@wimglenn
Copy link
Contributor Author

wimglenn commented Dec 9, 2022

Nested containers remain an ConfigNode instances after mutation, reproducer:

>>> cfg = Config({"a": {"k1": [], "k2": {}}})
>>> cfg.a = dict(sorted(cfg.a.items(), reverse=True))
>>> cfg.data
{'a': {'k2': configurator.node.ConfigNode({}), 'k1': configurator.node.ConfigNode([])}}

@cjw296
Copy link
Member

cjw296 commented Dec 12, 2022

Yeah, I feel like this is a tough one. Like we talked about: configurator should arguably strip off the ConfigNode wrapper at assignment time, but there will likely be cases where this doesn't work (assigning a dict mapping to ConfigNodes, etc).

I feel like the answer might want to be "you should be using the mapping/merging APIs" but want to leave this open for the next time I have a proper chance to play with this project...

@cjw296 cjw296 changed the title Value-based equality comparisons Assignments should strip off any ConfigNode wrappers. Dec 12, 2022
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