Kijewski / pyjson5

A JSON5 serializer and parser library for Python 3 written in Cython.
https://pyjson5.readthedocs.io
Apache License 2.0
143 stars 8 forks source link

supply_bytes=True in encode_* brings a headache to me #69

Open SyberiaK opened 1 year ago

SyberiaK commented 1 year ago

Hi there, I'm currently working on personal l10n system that can use different JSON parsing implementations. It looks something like this:

import json
from types import ModuleType

class L10n:
    def __init__(json_impl: ModuleType = json)
        self.json_impl = json_impl

    def load(file):
        self.json_impl.load(file, indent=2, ensure_ascii=False)

    def dump(data, file):
        self.json_impl.dump(data, file, indent=2, ensure_ascii=False)

Of course, we are expecting the same interface as builtin json have.

Because pyjson5.dump(...) and other encode functions have supply_bytes=True, it throws an error:

  File "C:\Users\User\PycharmProjects\l10n\sl10n\process.py", line 93, in apply_postmodifiers
    self.json_impl.dump(data, f, indent=2, ensure_ascii=False)
  File "src/_legacy.pyx", line 100, in pyjson5.pyjson5.dump
  File "src/_exports.pyx", line 562, in pyjson5.pyjson5.encode_io
  File "src/_encoder.pyx", line 439, in pyjson5.pyjson5._encode_callback_bytes
  File "src/_encoder.pyx", line 422, in pyjson5.pyjson5._encode
  File "src/_encoder.pyx", line 381, in pyjson5.pyjson5._encode_other
  File "src/_encoder.pyx", line 232, in pyjson5.pyjson5._encode_mapping
  File "src/_writer_callback.pyx", line 9, in pyjson5.pyjson5._WriterCbBytes_append_c
TypeError: write() argument must be str, not bytes

I know that changing defaults would break old code, so is there any way we can fix this?

SyberiaK commented 1 year ago

Nevermind, i realized that dumping json5 doesn't make much sense as decoding doesn't load comments (which is kind of point I wanted to use json5 as an option). Anyway, leaving this issue for discussion.

chaseastewart commented 10 months ago

+1 Would be nice if dump supports both string and bytes, its not very clear in the documentation that only bytes is supported. Kind of a shocker when changing from the default json impl.

dimateos commented 10 months ago

This makes very common samples like How to dump a dict to a JSON file? (SO) throw the same exception.

Therefore, the following does is not really true:

The library supplies load(s) and dump(s) functions, so you can use it as a drop-in replacement for Python's builtin json module, but you should use the functions encode*() and decode*() instead.

The lib should load/dump the same data if I just use it as a replacement for a regular json...