Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Nested dicts #3

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
2 participants

ajweb commented Mar 16, 2013

Hi hugh,
I've finally had some free time to do this!

When I first used your class for the project I was working on, I added the support for nested dicts in a very specialized way. I tried to do more or less the same I was doing but in a more general way and integrated into the DictDiffer class.

I'm sure it can be improved though. For a start, the format I'm using to return the differences is not very intuitive. Again, it made sense for my case, but a different way may be better for most people. Perhaps returning a tuple of all the keys up to, and including, the one changed, instead of separating the parents.

I'll try to set aside a bit more time next week to try to improve it, and I'm sure you and others will easily improve it as well.

Thanks again for your original code. First class.

Owner

hughdbrown commented Jun 17, 2013

I owe you an answer. I'd prefer not to take this code, if only because it is a totally different direction and I am not wild about the interface for returned differences. The use of tuples seems off for describing a dictionary hierarchy. If I were doing this, I might use strings with dot-notation, like this:

['c.b', 'c.x', 'c.e']

And I'd provide an accessor to get from dot-notation to the differing values in the dictionaries. Or I'd return the differences as a dictionary, keyed on :

{
    'c.b': (2, 3), 
    'c.e': (2, 3),
    'c.x': ({'y': {'h': 4}}, {}),
}

Or maybe use dot-notation consistently:

{
    'c.b': (2, 3), 
    'c.e': (2, 3),
    'c.x': ('y.h': 4, None),
}

But the alternative display requires some more thought.

By all means, go out and call this your own.

@hughdbrown hughdbrown closed this Apr 11, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment