Skip to content

add logic to convert dict list key to tuple #633

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

Conversation

jensbjorgensen
Copy link

Python dictionaries with tuples as a key can be packed, however when they are unpacked an error occurs because the key
comes back as a list rather than a tuple, although it's possible to set at the top-level use_list=False this is undesirable as
it affects everything. This solution is slightly wasteful as it results in a list being created and then converted to a tuple however it has the advantage of being the smallest change to the code that makes this work.

Comment on lines 532 to 533
if isinstance(key, list):
key = tuple(key)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is tuple the only data type that is valid as a dict key and that would come back as a list after pack/unpack?

Nitpick: use elif

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you could conceivably subtype list and then add eq and hash to make it eligible as a key, is that what you mean?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose that could be changed to type(key) is list instead of using isinstance? How do you like that?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I think that makes a lot of sense, if type(key) is list then the ret[key] = after that will definitely throw

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ThomasWaldmann are you ok with type(key) is list ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what I meant is research which data types msgpack will serialize into something that comes back as a list after pack/unpack.

because if that is more than just tuple and you are always casting it to a tuple, it is something else than it was before packing.

as you are only doing it in dict key context, it can't be a list, because that would not be valid as a dict key, but maybe it could be sth else?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, that is true. To speak concretely a value to be used as a key in a dict must be hashable, so it's easy to sub-type list and define hash and eq and make it valid as a key. Consider the following:

from __future__ import annotations
import msgpack

class MyList(list):
    def __hash__(self) -> int:
        return self[0]

    def __eq__(self, rhs: MyList) -> bool:
        return self[0] == rhs[0]

l0 = MyList((1, 2, 3, 4))
l1 = MyList((1, 2, 3, 4, 5))
d = {l0: 'one through four'}
d[l1] = 'one through five'
print(d)

data = msgpack.packb(d)
print(msgpack.unpackb(data)) #exception occurs here

The way I'm thinking about this though is that if you as the programmer have used an object which gets serialized as a list and you haven't already done something yourself to make sure when it is unpacked it gets unpacked as something that is not simply "list" then you will get an exception. One way you could've gotten around this already would be to define a pairs_hook. I thought the above would be more elegant. I guess the one scenario I could think of that my proposed change would break is if they had defined their own dict object that was willing to accept a plain list as a key. Would it be more palatable if I actually just went ahead and tried to make the assignment, check for an exception due to it not being a suitable key value and then convert it to a list?

@methane
Copy link
Member

methane commented May 19, 2025

It is not difficult to convert not only simple lists, but also nested list. (e.g. unpack(pack({ ((1,2),): None }))) Why not?
And if Python added frozendict to the standard library, it could support dict.

But I don't want to do that. msgpack is not a Python-specific format like pickle, but a programming language-independent format like JSON. Just because it's expressible in msgpack's wire format doesn't mean we have to implement additional complexity to support it. We don't want to add complexity to the library.

If the problem is that packer can pack tuples in dict key, how about adding a strict=True option to packer and only key bytes and strings, like unpacker does?

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

Successfully merging this pull request may close these issues.

3 participants