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

"\t" no longer supported as an indent #432

Closed
ZLJasonG opened this issue Oct 8, 2020 · 6 comments
Closed

"\t" no longer supported as an indent #432

ZLJasonG opened this issue Oct 8, 2020 · 6 comments
Labels
duplicate This issue or pull request already exists

Comments

@ZLJasonG
Copy link

ZLJasonG commented Oct 8, 2020

Python 3.7.6
ujson 4.0.0

import ujson
ujson.dump(output, f, indent="\t")
TypeError: an integer is required (got type str)

This behaviour has changed since ujson-3.2.0

@hugovk hugovk added the bug Something isn't working label Oct 8, 2020
@hugovk
Copy link
Member

hugovk commented Oct 8, 2020

Confirmed, here's a self-contained example:

import ujson

print(ujson.__version__)

# ok
ujson.dumps([{"key": "value"}])

# TypeError: an integer is required (got type str)
ujson.dumps([{"key": "value"}], indent="\t")

@hugovk
Copy link
Member

hugovk commented Oct 8, 2020

git bisect says 5b979ee is the first bad commit, from PR #426:

commit 5b979eeebf31f3bc8d5de914e578935cce8571e1
Author: Chen-Han Hsiao (Stanley)
Date:   Thu Sep 17 20:39:38 2020 +0800

    Fix indent and add test case

 python/objToJSON.c  | 2 +-
 tests/test_ujson.py | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

@chenhan1218, please could you check this? Thank you!

@ZLJasonG
Copy link
Author

ZLJasonG commented Oct 8, 2020

To be fair, it doesn't look like it actually worked before, it used a single space instead of the tab, it just didn't complain about it.

@hugovk hugovk removed the bug Something isn't working label Oct 8, 2020
@hugovk
Copy link
Member

hugovk commented Oct 8, 2020

Good point, and there's already an issue for it: #319 (edit: and #317).

The TypeError happened in version 2, and then commit fe0e88d from PR #327 (version 3.0.0) replaced it with spaces:

[
 {
  "key": "value"
 }
]

It would be good to support the same API as stdlib json:

If indent is a non-negative integer or string, then JSON array elements and object members will be pretty-printed with that indent level. An indent level of 0, negative, or "" will only insert newlines. None (the default) selects the most compact representation. Using a positive integer indent indents that many spaces per level. If indent is a string (such as "\t"), that string is used to indent each level.

https://docs.python.org/3/library/json.html#basic-usage

@chenhan1218
Copy link
Contributor

@hugovk
Yes. I could confirmed.
And this issue has also happened on v2.0.0, v1.34

As you mentioned, the issue is same as #319 .

( In v3.0.0, this TypeError issue doesn't happen but the behavior is also not correct. )

@hugovk
Copy link
Member

hugovk commented Oct 8, 2020

Thanks all, let's close this as a duplicate of #319.

@hugovk hugovk closed this as completed Oct 8, 2020
@hugovk hugovk added the duplicate This issue or pull request already exists label Oct 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

3 participants