Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DRAFT First try (NOW WORKING!) to migrate to Pydantic v2 #67

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

spirali
Copy link
Contributor

@spirali spirali commented Jan 29, 2024

This is my try to migrate to Pydantic v2 API.

I am now able to create a JSON schema from dataclasses but schema did not pass the tests (because of JSON reference solving?) and it is not clear to me what it should exactly do. @gavento can you check this please?

@spirali spirali requested a review from gavento January 29, 2024 11:55
@spirali
Copy link
Contributor Author

spirali commented Jan 29, 2024

This relates to #66

@gavento
Copy link
Contributor

gavento commented Jan 30, 2024

Thanks for the PR! I won't be able to dig in into the code in the next few days.

Some quick guesses based on the failed tests, though:

Based on the error:

>       assert sch == json.loads(B_SCHEMA_DEREF_FULL)
E       AssertionError: assert {'$defs': {'A...pe': 'object'} == {'properties'...pe': 'object'}

... it seems that json_schema.get_json_schema does not produce a "clean" json schema, but leaves some $defs and references. In my experience, LLMs are best at answering "dereferenced" JSON schemas. Maybe check what happens in deref_jsonref?

But based on the next error:

>       assert query_for_json_ex(test_model, Foo, "TEST_A") == Foo(z=False)
E       AssertionError: assert Foo(z='0', x=0, y=None) == Foo(z=False, x=0, y=None)

... something else may be going on as well with pydantic, as Foo.z is declared as having type bool in the dataclass, so it should never come out as str here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants