Weird indentation when using custom Encoder class

[Hirnmoder]

First of all, thank you for providing the json5 package. It really does improve the configuration file readability. So far, I have not encountered any issues while parsing / loading and un-parsing / dumping.
However, I did notice a strange behavior when using the indent setting during dumps(...). Please see this MWE below:

import json5

class Foo:
    def __init__(self):
        self.bar = Bar()
        self.baz = "hello"
        self.foobar = None
        self.arr = [1, 2, 3]
        self.qux = {"a": 1, "b": 2}
        self.obj = Bar()

class Bar:
    def __init__(self):
        self.answer = 42

class Encoder(json5.JSON5Encoder):
    def default(self, obj):
        if isinstance(obj, Foo):
            return {"__type": "Foo", **vars(obj)}
        if isinstance(obj, Bar):
            return {"__type": "Bar", "answer": obj.answer}
        return super().default(obj)

print(
    json5.dumps(
        Foo(),
        indent=4,
        cls=Encoder,
    ),
)

It gives this semantically correct output:

{
        __type: "Foo",
        bar: {
                __type: "Bar",
                answer: 42,
            },
        baz: "hello",
        foobar: null,
        arr: [
            1,
            2,
            3,
        ],
        qux: {
            a: 1,
            b: 2,
        },
        obj: {
                __type: "Bar",
                answer: 42,
            },
    }

As you can see, the indentation gets applied twice for my custom objects (Foo and Bar) and their closing braces do not line up with the (semantically) corresponding opening braces.

This is due to an additional level + 1 in _encode_non_basic_type.

In fact, overriding the method _encode_non_basic_type in the custom Encoder class will fix this issue:

# ...
class Encoder(json5.JSON5Encoder):
    def default(self, obj):
        if isinstance(obj, Foo):
            return {"__type": "Foo", **vars(obj)}
        if isinstance(obj, Bar):
            return {"__type": "Bar", "answer": obj.answer}
        return super().default(obj)

    def _encode_non_basic_type(self, obj, seen: Set, level: int) -> str:
        # Basic types can't be recursive so we only check for circularity
        # on non-basic types. If for some reason the caller was using a
        # subclass of a basic type and wanted to check circularity on it,
        # it'd have to do so directly in a subclass of JSON5Encoder.
        if self.check_circular:
            i = id(obj)
            if i in seen:
                raise ValueError("Circular reference detected.")
            seen.add(i)

        # Ideally we'd use collections.abc.Mapping and collections.abc.Sequence
        # here, but for backwards-compatibility with potential old callers,
        # we only check for the two attributes we need in each case.
        if hasattr(obj, "keys") and hasattr(obj, "__getitem__"):
            s = self._encode_dict(obj, seen, level + 1)
        elif hasattr(obj, "__getitem__") and hasattr(obj, "__iter__"):
            s = self._encode_array(obj, seen, level + 1)
        else:
            s = self.encode(self.default(obj), seen, level, as_key=False)    # <---- this line is the culprit
            assert s is not None

        if self.check_circular:
            seen.remove(i)  # type: ignore
        return s
# ...

Output:

{
    __type: "Foo",
    bar: {
        __type: "Bar",
        answer: 42,
    },
    baz: "hello",
    foobar: null,
    arr: [
        1,
        2,
        3,
    ],
    qux: {
        a: 1,
        b: 2,
    },
    obj: {
        __type: "Bar",
        answer: 42,
    },
}

[dpranke]

That looks like a bug alright. I haven't sat down at the keyboard yet to confirm it, and I'm not sure that's quite the fix I'd choose, but it's at least close. I'll try to get a fix out later today.

Thanks for reporting this and even suggesting a fix! :).

[Hirnmoder]

Thank you for your super-swift reply!
I'm not sure what the "correct" way of fixing this issue is, but thank you for looking into this matter. For me personally, it is not time-critical as it is just a minor flaw in the output JSON.

For those who do not like patching the JSON5Encoder, there is another way of getting rid of these weird indentations:

# ...

json5.dumps(
    json5.loads(
        json5.dumps( Foo(), cls=Encoder, indent=4 ) # actually, indent is not necessary here
   ),
   indent=4, # this is the final output indentation
)

Of course, this comes with a humongous performance hit due to dumping, loading, and dumping again.

[dpranke]

Sorry for the delay in fixing this; it should be good now w/ version 0.12.1, and I've pushed that version to PyPI.

[dpranke]

And yes, your fix was the thing to do :). Thanks again for reporting this!

[Hirnmoder]

Sorry for the delayed response from my side.
Thank you for fixing this bug and publishing the new version of the json5 package so quickly! Works as expected 🚀