rusackas commented on PR #38172:
URL: https://github.com/apache/superset/pull/38172#issuecomment-4432993126
@msyavuz Good question — I considered that path but it has a few catches:
1. **`stringify_values()` doesn't use JSON.** The hot path is
`obj.astype(str)` (numpy's per-dtype string cast, equivalent to Python's
`str()`), with `json.dumps` only as a fallback for objects that `astype(str)`
can't handle. So `{'a': 1}` becomes `"{'a': 1}"` (Python repr, single quotes,
not valid JSON), and `json.loads` on the way back would fail. Switching the
primary path to JSON would change the wire format for every nested column we've
ever produced — risky.
2. **`stringify_values()` has many callers.** It's the workhorse for
stringifying *all* columns PyArrow can't handle natively (Decimal, datetime,
custom types, etc.), not just JSON. A serialize/deserialize wrapper there would
either need to be type-aware (essentially re-implementing the current PR's "is
this a dict/list?" detection at a lower layer) or risk lossy round-trips for
non-JSON types.
3. **Round-trip fidelity.** Capturing the original Python objects preserves
things like `NaN` inside nested dicts, datetime values, etc., that JSON
serialization mangles or loses. The shadow `_nested_columns` map adds a
per-nested-column reference but keeps the original objects intact.
So the current PR's "snapshot before stringify, restore after pandas
conversion" is essentially the smallest blast-radius shim that hits the goal of
`#25125` without touching wire format or the broader stringification pipeline.
If you see a cleaner serialize path I'm missing — happy to revisit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
