-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix: add ensure_ascii=False to json.dumps for correct Unicode output #639
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
base: main
Are you sure you want to change the base?
Conversation
96a1aec
to
3b295e4
Compare
Thanks for the suggestion! You’re absolutely right. Non-English text in debug logs isn’t very developer-friendly at the moment. Generally speaking, as long as all input text data is UTF-8 encoded, this shouldn’t introduce any issues. In particular, the changes to the debug logging should be safe. That said, the changes in the following files could potentially affect behavior (though I think it’s unlikely). Still, it would be good to test with a few patterns just to be sure:
Also, the CI tests detect an issue with older Python versions. Could you resolve it? |
@seratch Regarding the following two files:
I realized afterward that the changes I made to them were unrelated to the main goal of this PR, which is to improve debug logs. I’ve removed those changes. I’ve also resolved the CI test failure on older Python versions. |
This PR is stale because it has been open for 10 days with no activity. |
6df3f88
to
f440664
Compare
51f0366
to
6b1db96
Compare
This PR is stale because it has been open for 10 days with no activity. |
@rm-openai |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Set
ensure_ascii=False
injson.dumps(...)
to allow proper Unicode character output (e.g., Japanese).Without this, non-ASCII characters are escaped (e.g., 日本語 → \u65e5\u672c\u8a9e), which makes logs or exports harder to read.