Repository navigation
ValueError with numeric keys #58
Description
JavaScript variables in the "wild" often have keys in numeric form. As an example, here's an extract of one from a webpage at www.sec.gov:
{"10-12B":"a|3|Registration of securities [Section 12(b)]",144:"a|2|Report of proposed sale of securities"}
Unfortunately this seems to choke the library:
>>> import json5
>>> json5.loads('{"10-12B":"a|3|Registration of securities [Section 12(b)]",144:"a|2|Report of proposed sale of securities"}')
ValueError: <string>:1 Unexpected "4" at column 61
Not sure if this is a library design choice (not mentioned in the docs) or an oversight, but thankfully in the interim the demjson3 library does work with them.
Activity
[-]Numeric keys[/-][+]ValueError with numeric keys[/+]dpranke commentedon Aug 8, 2022
Thanks for mentioning this issue!
Integer keys are legal in regular JavaScript, but they are not permitted in the json5 specification, that this library implements.
At the moment, the docs kinda assume that you're familiar with JSON5, but given how the docs are written now, I can certainly see how you might think that more stuff from JS was legal than is actually the case.
I'd be happy to update the docs somehow to make the prohibition clearer. If you have suggestions for how to do so, I'm happy to hear them. Or, I can take a shot at it, and I'd be fine with mentioning the
demjson3library as an alternative.I would be reluctant to change the code to allow integer keys without buy-in from the folks that own the JSON5 spec, because that might encourage people to create more illegal json5 docs. Alternatively, I could add an optional flag to allow them, but I wouldn't even really want to do that unless there was clear demand for this from many users.
Any thoughts on the above?
Side note: can you provide a full URL to wherever you're finding that snippet of code? I didn't actually see that string in the source of https://www.sec.gov, so I wasn't sure if you were saying it was literally on that page, or just somewhere on the SEC site.
mborsetti commentedon Aug 8, 2022
Hello @dpranke,
Thanks for a comprehensive response! I indeed looked for a specification for JSON5 to understand where the issue was, but it points to ECMAScript 5.1 and IdentifierName, which I never heard of, and stopped the search there to avoid entering a rabbit hole.
My motive was to consolidate dependencies after finding
json5from being loaded by another dependency. I saw in the docs the reference to JavaScript, so I replaceddemjson3(which does not seem to support JSON5, at least from the docs) with it, and it worked fine and forgot about it, until by chance it had to parse thesubmissionFormsvariable at https://www.sec.gov/edgar/browse/scripts/lookup-data.min.js and got this Exception.It's your project so I can't really advise as to how you want to proceed, but having a universal tool to read (not necessarily write) JSON, JSON5 and "in the wild" JavaScript JSON would be super useful.; if you want to maintain purity, one way is indeed to add an optional flag, or, even better, issue a warning and add a flag to suppress it. If not, then I would definitely advise editing the docs to be explicit that it does not support "in the wild" JavaScript JSON.
Thanks!
dpranke commentedon Aug 18, 2022
Whoops, sorry for going radio silent on you :(.
I think at the moment I'm inclined to not add anything to support this. This library is definitely not intended to be used to parse regular JavaScript, and that's what the stuff in that link is.
If others can come up with good use cases for this, I'm happy to revisit the issue.
Does that make sense? Sorry for not wanting to support what you're doing.
mborsetti commentedon Aug 18, 2022
No problem and thanks. Your project, your rules, am cool with it! I do would like to suggest that you remove/rephrase the reference to JavaScript on the homepage so that others don't make the wrong conclusion like I did!
dpranke commentedon Aug 18, 2022
That's a good suggestion about the docs, I'll add that. Thanks!
Try to improve docs to address issue #58.
dpranke commentedon Aug 18, 2022
Done in e28f7b9 / 0.9.10.