Skip to content

Conversation

@sydney-runkle
Copy link
Contributor

@sydney-runkle sydney-runkle commented Jan 21, 2025

@codspeed-hq
Copy link

codspeed-hq bot commented Jan 21, 2025

CodSpeed Performance Report

Merging #1602 will not alter performance

Comparing fix-initvar-required (3e82624) with main (b426cce)

Summary

✅ 157 untouched benchmarks

@sydney-runkle sydney-runkle marked this pull request as ready for review January 21, 2025 18:53
Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We definitely need some kind of test for this change of behaviour!

We can manually build the core-schema, but we absolutely need a test or tests.

Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otherwise I think LGTM.

)

s = SchemaSerializer(schema)
assert s.to_python(Foo(x=1), warnings='error') == {'x': 1}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please can you add a test with just the Foo schema, and a test for Bar here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and can you add a test for Foo(x=1, init_var=2)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please can you add a test with just the Foo schema, and a test for Bar here.

Not quite sure what you mean here - I wasn't able to repro the issue without the union.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still it's still worth adding tests for the simplest cases.

Copy link
Contributor

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@davidhewitt davidhewitt merged commit 261c7ab into main Jan 23, 2025
29 checks passed
@davidhewitt davidhewitt deleted the fix-initvar-required branch January 23, 2025 19:06
davidhewitt pushed a commit to pydantic/pydantic that referenced this pull request Oct 20, 2025
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.

Dataclass with InitVar and Union gives warning during serialization

4 participants