BUG: Attributes skipped when serialising plain Python objects to JSON (#42768)

[joelgibson]

This patch closes #42768 and fixes a previously-reported issue #33043.

When ujson falls back to encoding a Python object, it iterates over all of the non-callable attributes of the object which do not start with an underscore, using them as JSON keys. While iterating the dirs(obj) list, an index was incremented twice causing every second attribute to be skipped.

I've fixed this increment, and moved some of the declarations inside the function so that control flow is more clear. I've also added a regression test for this behaviour.

• closes BUG: to_json() swallows attributes of dataclasses #42768
• tests added / passed
• Ensure all linting tests pass, see here for how to run them
• whatsnew entry

This patch also xref #41174, but I'm not sure that turning complex numbers into {"real": ..., "imag": ...} JSON objects was intended in the first place, so I haven't added a test for this.

[jreback]

does this have any user visible effects in to_json itself?

[jreback]

can you replicate the test in the OP

[joelgibson]

@jreback The test more or less duplicates the OP, just using regular classes rather than dataclasses. However, the expected output in the OP included properties beginning with underscores, wheras I have left these out of the resulting JSON because this is what the existing (intended, I assume) behaviour did.

It does have user-visible effects, for example if classes are stored as elements in series or dataframes. The program:

import pandas as pd

class A:
    def __init__(self, a, b):
        self.a = a
        self.b = b

series = pd.Series([A(a=1, b=2)])
print(series.to_json())

used to return {"0":{"a":1}}, and now returns {"0":{"a":1,"b":2}}.

[mzeitlin11]

Thanks for the pr @joelgibson! While I agree that the changes you made make the code more readable, would it possible to leave those for a followup pr? Just to make the changes that actually affect code logic clearer

[jreback]

some test comments, ping on green.

[joelgibson]

Thanks @mzeitlin11 - I would usually only change the part of the function which affected the logic, but the logic of that function was so unclear it seemed to warrant a cleanup anyway. For example on 928 it looks like something interesting is being assigned but actually NULL is being assigned. (Incidentally, how should I go about putting in the cleanup PR - just title it with "CLN" and reference this discussion?).

@jreback ping

[mzeitlin11]

LGTM, thanks @joelgibson! Followup with your original cleanups would be great!

(Incidentally, how should I go about putting in the cleanup PR - just title it with "CLN" and reference this discussion?)

Yep sounds good, nothing special needs to be done

[jreback]

thanks @joelgibson