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

optional merge strategies for lists/sets #10

Open
kbroughton opened this issue Jun 14, 2016 · 6 comments
Open

optional merge strategies for lists/sets #10

kbroughton opened this issue Jun 14, 2016 · 6 comments

Comments

@kbroughton
Copy link
Collaborator

The standard library defines what is meant by update for two dicts.
For other container types, there may be several options.
Currently on d1.update(d2) if the value is a list then d2's list over-writes d1's list.

I would like to have the option of specifying the behaviour for lists and sets.
For example, i would like the lists here to merge by appending
d1 = {'a':1,'f':[1,3]}
d2 = {'a':1,'f':[1,2]}
d1.update(d2) ->
{'a':1,'f':[1,3,1,2]}

Or i might prefer my lists to uniquely append
d1.update(d2) ->
{'a':1,'f':[1,3,2]}

Or as in this S.O. http://stackoverflow.com/questions/1319338/combining-two-lists-and-removing-duplicates-without-removing-duplicates-in-orig

This could be done if update took a strategies parameter, perhaps a dict mapping types to combiners.
strategies=[{'name': 'unique_extend_list', 'signature': ('list','list'): 'combiner': lambda: x,y: x + list(set(y) - set(x)) },]
And then modify the _recursive_update to handle the cases.

Have you considered something like this?

@kbroughton
Copy link
Collaborator Author

I have a pr for this if you are interested

@bunbun
Copy link
Owner

bunbun commented Jun 14, 2016

I would be very interested, provided of course it does not (unduly? !!) complicate the interface / steepen the learning curve for the average user.
Thank you very much!

@kbroughton
Copy link
Collaborator Author

great. Could you add me to the repo collaborators or check it out at
https://github.com/kbroughton/nested-dict/blob/optional-merge-strategies/nested_dict/implementation.py
For some reason i can't create a normal PR from my branch to your repo.

The new functionality/interface should be completely backwards compatible. If you like it, i can add tests and docs.

@bunbun
Copy link
Owner

bunbun commented Jun 14, 2016

I have added you to the list of repo collaborators.
However...

:-)

I think it is a little complicated as it stands.

Would it help to

  1. rename "strategy" to "combine_policy" or something?
    Is "strategy" a python-y thing. I am more familiar with c++ (traits and) policies. Or is this GOF inspired?
  2. Think of some way for users to add policies above using duct typing, for example by adding an extra parameters to update?

Would you also mind providing some examples and docs?
(And if you are the TDD sort of person... though that would be asking a lot!!!!)

P.S. Do you use nested dict a lot and did you have motivating test cases?

@kbroughton
Copy link
Collaborator Author

TDD? what's that?

  1. I'm fine with combine_policy rename. strategy was GOF inspired, but only vaguely.
  2. I haven't wired up the combine_policy_options yet but was planning to change the update signature
    def update(self, other, strategies=[], combine_policy_options):
    where combine_policy_options would add to the default policy_options.

I've been needing something like nested_dict for a while. I started working on something like it to allow dict-like access to lists or vice, versa, but got busy. I like your structure better anyway.

@kbroughton
Copy link
Collaborator Author

kbroughton commented Jun 15, 2016

I think you could make an argument that even the dict merge should be customizable.

I have a use case where I have a dict d1 and I'd like to over-ride it with d2 when their keys overlap, but not inject any items of d2 that aren't already in d1.

If we could over-ride dict combine policy then I could call

d1 = {'a':1,'f':'default'}
d2 = {'b':1,'f':'over-ride'}
d1.update(d2, combine_policies=['intersect_update']) ->
{'a':1,'f':'over_ride'} # note that d2's 'b' did not get inserted.

The combiner policy would be a bit more challenging in this case. Your thoughts? I would make this a separate PR though.

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