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

Dicts with non-string keys get serialized wrong #4

Open
LaCuneta opened this issue Sep 20, 2018 · 3 comments
Open

Dicts with non-string keys get serialized wrong #4

LaCuneta opened this issue Sep 20, 2018 · 3 comments

Comments

@LaCuneta
Copy link
Contributor

Copied from with more discussion at: qiemem/PythonExtension#6

Some examples:

observershow py:runresult "{1: 2}"
observer: [["1" 2]]
observershow py:runresult "{(1,2): 3}"
Extension exception: keys must be a string

While JSON is restricted to string keys, we are not. We need to hijack JSON's dict serialization to turn them into a list of pairs instead.

@arthurhjorth
Copy link
Member

Just want to make one note on this that isn't in the original issue/discussion:

This suggestion would turn tuples, which are hashable, into lists, which are not. This means that if someone reports a dictionary that has tuples as keys to NetLogo, and then pass them right back into python, it will fail because the keys are now unhashable lists. It would be quite confusing to people unless they are aware of this issue. (To be fair, most people who work with Python are...)

One option would be to iterate over keys in the extension, and turn all keys that are unhashable into a hashable object before creating the dictionary in python. But it seems a bit dangerous to make assumptions about what people want their data types to be, too.

@qiemem
Copy link
Member

qiemem commented Feb 7, 2020

Good call. Worth noting we don't attempt to convert anything to dictionaries right now. e.g.:

observer> py:set "foo" py:runresult "{1 : 2}" py:run "print(foo)"
[['1', 2]]

It's up to the user to then wrap foo in a dict.

So the only real option I see is to make tuples are default iterable target instead of lists, but that doesn't seem reasonable to me.

@arthurhjorth
Copy link
Member

I definitely agree with that, and it makes sense to just make people be explicit about calling dict() or tuple() when passing data back to Python.

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

3 participants