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

Allow indent to be a str or None for compatibility with Python's json #517

Open
Erotemic opened this issue Apr 5, 2022 · 2 comments · May be fixed by #518
Open

Allow indent to be a str or None for compatibility with Python's json #517

Erotemic opened this issue Apr 5, 2022 · 2 comments · May be fixed by #518

Comments

@Erotemic
Copy link
Contributor

Erotemic commented Apr 5, 2022

This is a feature request to make ujson slightly more compatible with Python's json module.

The current version of ujson expects that indent is always specified as an int, otherwise it will raise a TypeError.

However, Python's json module allows the user to specify indent as either an int, None, or a string. In Python, you can specify a string indent as something with non-space characters, but because of the way ujson is implemented, it would be harder to support that without a major refactor. As a compromise, it would be nice if ujson could accept indent as a string, verify that it only contains spaces, and then convert the number of spaces to an int for it's own internal usage.

With Python's json

import json
print(json.dumps([1], indent='    '))
print(json.dumps([1], indent=4))
print(json.dumps([1], indent=0))
print(json.dumps([1], indent=None))

Prints:

[
    1
]
[
    1
]
[
1
]
[1]

But in ujson

import ujson
try:
    print(ujson.dumps([1], indent='    '))
except Exception as ex:
    print(ex)
try:
    print(ujson.dumps([1], indent=4))
except Exception as ex:
    print(ex)
try:
    print(ujson.dumps([1], indent=0))
except Exception as ex:
    print(ex)
try:
    print(ujson.dumps([1], indent=None))
except Exception as ex:
    print(ex)

prints

'str' object cannot be interpreted as an integer
[
    1
]
[1]
'NoneType' object cannot be interpreted as an integer

I have a PR that will resolve this issue.

@Erotemic Erotemic linked a pull request Apr 5, 2022 that will close this issue
@JustAnotherArchivist
Copy link
Collaborator

I think it's worth going for full compatibility even though that requires a significant refactor. I wanted to look into this a while ago, but it got blocked by the buffer overflow issues (cf. #504) since it requires proper memory management.

@Erotemic
Copy link
Contributor Author

Erotemic commented Apr 5, 2022

The refactor wasn't as difficult as I thought it was, so I added full indent support in #518 but I'm not sure if there are memory management issues. It would be good if someone could vet the code.

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 a pull request may close this issue.

2 participants