msgpack / msgpack-python

MessagePack serializer implementation for Python msgpack.org[Python]
https://msgpack.org/
Other
1.92k stars 230 forks source link

change tuple as dict key behavior of strict_map_key=False use_list=True in unpackb #422

Closed ssolari closed 4 years ago

ssolari commented 4 years ago

This may only be a question for discussion, but personally I think the default behavior of unpackb under the condition where strict_map_key=False and use_list=True should work differently.

The following will throw an error:

d = {(1, 2, 3): 'tuple_key'}
packed_bytes = msgpack.packb(d, use_bin_type=True)
unpacked_dict = msgpack.unpackb(packed_bytes, strict_map_key=False, use_list=True)

I understand that use_list=True implies that all returned lists should be 'lists'. However, it can NEVER be the case that a mutable (unhashable) object should ever be the key to a dictionary. In the case with strict_map_key=True, of course this should throw an error, however, when strict_map_key=False it is implied that there will be other types of hashable (immutable) keys, like a tuple.

I think in this one case it makes a lot of sense to automatically convert any 'list/tuple' to a tuple if it is a key (how could it even have been passed to packb if it wasn't a tuple in the first place?). Then all other lists would be returned as lists. In our usage of msgpack, we always want to return use_list=True, but in our use case there can be tuples as keys to dictionaries. So this forces us to always pass use_list=False, which requires checking and conversion of all returned tuples back to lists.

I think changing this behavior could be beneficial to many use cases and I don't see how changing this default behavior would cause issues (backward or forward) nor confusion, but maybe i'm missing something as well.

methane commented 4 years ago

See this comment: https://github.com/msgpack/msgpack-python/issues/418#issuecomment-602014948

I don't think the usage is common enough to introduce more complexity in this library. For example, people use only string key in JSON objects. (Note that msgpack is JSON-like format).

ssolari commented 4 years ago

@methane thanks for the response. I get the desire to not introduce complexity. I will say that msgpack is far better than Json so it probably should have a few more flavors!;). In #418 you mentioned using object_hooks. Could you elaborate at all on how that would be used to identify the keys and keep them as tuple when use_list=True?

jfolz commented 4 years ago

@ssolari define a object_pairs_hook to convert all keys that are list to tuple, then create and return the dict.