feat: serialize pathlib paths like the other extra types - #1962
feat: serialize pathlib paths like the other extra types#1962shcheklein wants to merge 1 commit into
Conversation
Deploying datachain with
|
| Latest commit: |
934e119
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3d3b2bef.datachain-2g6.pages.dev |
| Branch Preview URL: | https://fix-json-encode-path.datachain-2g6.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
97ba22e to
9a8c415
Compare
datachain.json already teaches ujson the types it does not handle on its own --
datetime, date, time, UUID, numpy arrays, bytes -- so that values reaching it as
Python objects can still be written. A path was not among them, so a model field
declared as Path fails to serialize once its value arrives unconverted:
TypeError: Object of type PurePosixPath is not JSON serializable
That is reachable today through a list of models, whose fields are dumped in
Pydantic's python mode and so keep their Python types.
Paths are written with str(), which is the spelling Pydantic's JSON mode
produces for PurePath, PurePosixPath and Path alike, so a value written through
either route reads back the same.
Paths stay refused as mapping keys where _to_jsonable builds the mapping, which
is what happened before by way of the encoder not knowing the type. A key is
written as its JSON text, quotes and all, and nothing on the read side turns
that back into a path, so accepting one would store a name no query matches.
Array items are a separate matter and are left alone: whether one reaches
_to_jsonable at all already differs by backend, which has nothing to do with
paths. Rich mapping keys need read-side reconstruction to work anywhere, for
paths as much as for datetimes, and that is left alone here too.
9a8c415 to
934e119
Compare
|
Closing. Path was never the goal here — it entered as a regression of my own change. The chain was: #1943's xfail needs live models to reach the converter as plain dicts, so I proposed flattening tuples like lists; that used python-mode It also does not earn its place on its own. Measured against main:
One row fixed, one row turned from a loud failure into silent type loss. Not a good trade for three lines. The useful finding is why: type support is spread across |
A model field declared as a
Pathcannot be stored:datachain.jsonexists to teach ujson the types it does not handle on its own —datetime,date,time,UUID, numpy arrays,bytes. Paths were missing, so a value that reaches the encoder still holding its Python type has nowhere to go.That happens for a list of models, whose fields are dumped in Pydantic's python mode and therefore keep their Python types. The same model reached another way is converted by Pydantic first and stores fine, which is why this shows up in one shape and not another.
Paths are written with
str(), which is what Pydantic's JSON mode produces forPurePath,PurePosixPathandPathalike, so a value written either way reads back identically. The test asserts that equivalence againstmodel_dump(mode="json")rather than hard-coding a string.Scope: values, not keys
Teaching the encoder about paths made one shape reachable that
mainrejected, and it stored badly — the key became its JSON text, quotes and all, the two read routes disagreed, and an equality filter matched nothing. So a path used as a mapping key is refused explicitly where_to_jsonablebuilds the mapping, keepingmain's behaviour:This covers a path key wherever
_to_jsonablebuilds the mapping, which is every JSON column on both backends. It does not reach array items, whose handling already differs by backend for reasons unrelated to paths — that is untouched here.Making rich mapping keys work — paths, datetimes, UUIDs — needs read-side reconstruction and filter normalisation, and none of them work today. That belongs with the read-path work in #1918.
Both encoder branches are covered:
ujsonsilently coerces a path key while stdlib rejects it, so the tests assert path values work underserialize_byteseither way.