feat: encode the remaining types Pydantic's JSON mode knows - #1966
Closed
shcheklein wants to merge 1 commit into
Closed
shcheklein wants to merge 1 commit into
shcheklein wants to merge 1 commit into
Conversation
datachain.json teaches ujson the types it cannot write on its own, and covered
datetime, date, time, UUID, numpy and bytes. Pydantic's JSON mode covers more
than that, so a value that arrives still holding its Python type has nowhere to
go and the write fails:
TypeError: Object of type PurePosixPath is not JSON serializable
Which of the two converts a model decides whether that happens, and that is
settled by the shape the model sits in rather than by anything about the value:
a model inside a list is dumped in Pydantic's python mode, leaving its fields as
Python objects for this encoder, while the same model inside a tuple reaches the
warehouse live and is dumped in JSON mode instead.
Adds timedelta, PurePath, IPv4Address, IPv6Address, plain Enum and set, each
written the way Pydantic's JSON mode writes it, asserted against model_dump for
every one rather than against a fixed string. All six raise today, so nothing
that currently writes changes.
Decimal is left alone: it already writes, as a number where Pydantic writes a
string, so aligning it would change stored data rather than add to it.
Deploying datachain with
|
| Latest commit: |
44a4199
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e45487c1.datachain-2g6.pages.dev |
| Branch Preview URL: | https://feat-json-encoder-extra-type.datachain-2g6.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
Author
|
Closing — superseded by a different direction. This taught the encoder the types Pydantic's JSON mode already knows, so that python mode could be used everywhere. We are going the other way instead: use Pydantic's JSON mode everywhere, which covers those types natively and makes this unnecessary. What that direction needs instead is numpy normalized before the dump, since Pydantic refuses Keeping the measurement here for whoever picks it up — declared field types, JSON mode versus python mode plus this encoder:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A model field holding one of several ordinary types cannot be stored:
Why it depends on where the model sits
Two things can turn a model into storable JSON, and they know different types:
flattenmodel_dump()— python modemodel_dump(mode="json")Which one runs is decided by the container:
So the same field works in one shape and fails in the other. Measured, for a declared field type:
This PR closes five of those rows, plus
IPv6Address. Each is written the way Pydantic's JSON mode writes it, and the tests assert that equivalence againstmodel_dump(mode="json")rather than against a fixed string, so they stay honest if Pydantic changes.Scope
All six raise today, so nothing that currently writes changes. Eight of the nine new cases fail against
main.Decimalis deliberately left out: it already writes, as a number where Pydantic writes a string (1.2vs"1.20"), so aligning it would change stored data rather than add to it. That belongs in its own change.This is a prerequisite for making the two converters agree — with the type gap closed, which one runs stops changing what gets written.