inveniosoftware / dictdiffer

Dictdiffer is a module that helps you to diff and patch dictionaries.
https://dictdiffer.readthedocs.io
Other
838 stars 93 forks source link

Wrong patch format WRT the standard RFC 6902 #96

Open rikirenz opened 7 years ago

rikirenz commented 7 years ago

This is the test that I run

self.root= {}
self.head= {'foo': 'baz1'}
self.update= {'foo': 'baz2'}

non_list_merger = Merger(self.root, self.head, self.update, {})
try:
    non_list_merger.run()
except UnresolvedConflictsException as e:
    print(e.content)

This is the result of the previous code:

[Conflict(('add', '', [('foo', 'baz1')]), ('add', '', [('foo', 'baz2')]))]

In according to the standard RFC 6902 that defines: a JSON document structure for expressing sequence of operations to apply to a JavaScript Object Notation (JSON) document; (https://tools.ietf.org/html/rfc6902)

IMHO the result is wrong for 2 reasons:

  1. It is not wrapped in an object (but is a minor issue, tuples are fine)

  2. The format of the response is wrong because is not returning the key foo in the right place. This force users to handle different cases and build manually the path for a given patch.

lnielsen commented 7 years ago

Thanks for the suggestions. I don't think dictdiffer was ever meant to generate patches according to JSONPatch, and dictdiffer also diffs more than just JSON.

You could add a new feature to support output in JSONPatch, but not sure how much work is involved with that.

mikaelho commented 5 years ago

Is the current patch format documented in any other way except in the tests? I am thinking of some kind of a grammar that we adhere to.

jirikuncar commented 5 years ago

It's part of the diff doctring https://github.com/inveniosoftware/dictdiffer/blob/master/dictdiffer/__init__.py#L47-L49

mikaelho commented 5 years ago

I agree that the docstring contains a number of excellent examples, but it still leaves it to the reader to infer what the rules are, especially as it is all plain values with no keys.

I am thinking of something simple but explicit like this:

If we can make sure that the above is correct and complete, I can PR it to the docstring.

And for some reason, I really do not "get" path_limit. A description would add value there as well.

jirikuncar commented 4 years ago

@mikaelho the PR with a description from your comment is welcome 🙏