dpath-maintainers / dpath-python

A python library for accessing and searching dictionaries via /slashed/paths ala xpath.
MIT License
610 stars 91 forks source link

Should use `Mapping`, `Sequence` rather than `Mutable` versions #78

Open abarnert opened 6 years ago

abarnert commented 6 years ago

dpath (after #35 was fixed) distinguishes between mappings, sequences, and other objects using MutableMapping and MutableSequence, instead of Mapping and Sequence. This means that it can't search within immutable sequences, like tuples:

>>> dpath.util.get([1,2,3], '1')
2
>>> dpath.util.get((1,2,3), '1')
KeyError: '1'

What about functions that need to mutate the input, like set or add? I think that even there, you want to path through using Mapping and Sequence. You may need to check at the "bottom" level, where you do the update/insert, that it's actually a MutableMapping or MutableSequence, but I'm not sure even that is necessary—and if it is, you probably still want to check Mapping or Sequence first, so you correctly report that a tuple is immutable instead of reporting that it's not something you can path into.

Consider:

>>> x = [[]]
>>> dpath.util.new(x, '0/0', 'hi')
>>> x
[['hi']]
>>> x = ([], )
>>> dpath.util.new(x, '0/0', 'hi') # should succeed and give us (['hi'],)
TypeError: Unable to path into elements of type ([],) at []
>>> x = [()]
>>> dpath.util.new(x, '0/0', 'hi') # should fail because x[0] is immutable
TypeError: Unable to path into elements of type () at [0]

(Also, the text of the TypeError is a bit weird—the problem is pathing into elements of (), or of type tuple, not of type ().)


This does raise the question of whether you want to allow pathing into strings, since isinstance(str, Sequence). If you don't want that, you'd need to explicitly exclude it with a second isinstance which checks (unicode, bytearray) for 2.x, (bytes, bytearray) for 3.0-3.4, and ByteString for 3.5+. But maybe it's fine for dpath.util.get(['abc'], '0/0') return return 'a' instead of raising a TypeError about being unable to path into strings.

akesterson commented 4 years ago

This is a good discussion but it could alter the interface enough that we aren't comfortable putting in 1.0. The 2.x branch is internally improving on the 1.x branch and making maintenance and certain features easier, but its interface is still compliant with 1.0. So this is slated for 3.x unless there is great need for it.

Admittedly, this is an unspoken design constraint based on dpath's original (way back when) use-case; processing JSON, which does not have tuples. We did not actually consider all possible python objects for traversal, though we did try to remain conscious of its use case outside of JSON specific use cases.

akesterson commented 4 years ago

Blocked by #136