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

Converting dictionaries with the same integer and string keys produce multiple keys in the json #77

Open
balazs-endresz opened this issue Aug 27, 2013 · 8 comments

Comments

@balazs-endresz
Copy link

Hi

Sorry if this issue has been raised before I couldn't find another ticket about this.
The problem I come across is that the following produces invalid json:

> simplejson.dumps({1: "int", "1": "str"})
'{"1": "str", "1": "int"}'

http://tools.ietf.org/html/rfc4627#section-2.2

"An object structure is represented as a pair of curly brackets surrounding zero or more name/value pairs (or members). [...] The names within an object SHOULD be unique."

I'm not sure what would be the best way to deal with this, the simplest thing I can thing of is to add an option to raise an exception if the generated json string has multiple keys in it. But I suppose mentioning it in the docs would suffice as well.

Cheers
Balazs

@etrepum
Copy link
Member

etrepum commented Aug 27, 2013

I expect that this error checking feature would be expensive to implement (both in runtime cost and lines of code). I think the most sensible thing to do would be to just mention this in the docs, or do nothing. In the past ~8 years, this is the only time this potential issue has ever been brought up here. It's not technically invalid JSON, but the decode semantics aren't well defined by the spec.

@balazs-endresz
Copy link
Author

I don't really see what difference it makes that no one has raised this issue before, and it does technically produce invalid JSON. But I agree that changing the code because of this might not be ideal. But for me this was enough to cause dozens of test cases to fail which apparently relied on the order the keys were modified and now they are returning a different value for the same key than they did before.

@etrepum
Copy link
Member

etrepum commented Aug 27, 2013

The fact that this potential issue has been possible since the beginning, and nobody else has been bothered by it, is very relevant.

Not following a "SHOULD" doesn't make it invalid JSON. It is still valid. Parsers and validators will accept it.

Mixing strings and integers for your dictionary's keys sounds like a bad idea, and I don't recall when I've ever seen code do that. What's your use case for mixing them?

@balazs-endresz
Copy link
Author

I wasn't mixing the keys on purpose, there's a set of pre-defined integer values which get converted to keys in the json. Here's an example:

> model.json_field[1] = True
> model.save()
> model.json_field[1] = False
> model.json_field
'{"1": True, 1: False}'
> model.save()
> model.json_field
this can return either '{"1": true}' OR '{"1": false}' depending on the order the keys were modified

I know parsers can handle multiple keys I was just pointing out that this can produce ambiguous results.

@etrepum
Copy link
Member

etrepum commented Sep 2, 2013

Arguably the best solution to this problem would be to have an option that allows you to turn off the automatic coercion to string as object keys. This way you'd always get an error if you were doing something questionable in your model code, not just when there's a conflict.

@balazs-endresz
Copy link
Author

I agree, that'd be even better.

@nirs
Copy link

nirs commented Nov 13, 2015

JSON objects keys must be only strings,this should raise ValueError or TypeError:

json.dumps({1: "x"})

This behavior smells like php to me :-)

@cyberfox1
Copy link

JSON objects keys must be only strings,this should raise ValueError or TypeError:

json.dumps({1: "x"})

This behavior smells like php to me :-)

It smells like code-smell. Automatically converting my values to something else sound like the kind of "helpfulness" I don't need or want.

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

4 participants