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

coerce_numbers_to_str needs a per-field variant #8383

Closed
4 of 13 tasks
jkugler opened this issue Dec 15, 2023 · 10 comments · Fixed by #9137
Closed
4 of 13 tasks

coerce_numbers_to_str needs a per-field variant #8383

jkugler opened this issue Dec 15, 2023 · 10 comments · Fixed by #9137

Comments

@jkugler
Copy link

jkugler commented Dec 15, 2023

Initial Checks

  • I have searched Google & GitHub for similar requests and couldn't find anything
  • I have read and followed the docs and still think this feature is missing

Description

We are migrating our code from Pydantic 1 to 2, and came across a situation where we do want some fields to coerce numbers to strings, but other fields in the model should not be coerced. As far as I can tell from the docs, coerce_numbers_to_str is an all-or-nothing thing, and will affect the entire model. Being able to specify fields would be nice. Maybe something like:

model_config = ConfigDict(coerce_numbers_to_str="field1,field2,field3")

I don't know if that's the best syntax, but that would serve our purposes.

If there is a way to work around this feature not being there, could it be added to https://docs.pydantic.dev/latest/api/config/#pydantic.config.ConfigDict.coerce_numbers_to_str ?

Affected Components

@sydney-runkle
Copy link
Member

@jkugler,

Seems like a reasonable feature request 👍. I think we should still stick with True and False as the allowed settings, but we could support Field(..., coerce_numbers_to_str=True) or something along those lines.

@jkugler
Copy link
Author

jkugler commented Dec 16, 2023

Seems perfectly reasonable. I'm a Pydantic newbie thrown in the deep end by migrating a project from v1 to v2, so will quicly admit not being fully up to speed with the API.

@sydney-runkle
Copy link
Member

@jkugler,

No worries. Let us know if there's anything else you need help with in terms of your migration, or if you notice anything missing from the migration guide.

@NeevCohen
Copy link
Contributor

Hey @sydney-runkle, I was looking into this issue and I think I can work on this. I looked at the StrValidator in core, and saw that it only reads the coerce_numbers_to_str configuration from the config. My thoughts were to add a coerce_numbers field to the str_schema and make StrValidator read that value from the schema as well as from the model config.

@sydney-runkle
Copy link
Member

@NeevCohen,

Sounds great! Go for it - I think you have the right approach :). Maybe we stick with coerce_numbers_to_str as the name of the flag just to be consistent. Prioritize the field level setting, of course :).

@NeevCohen
Copy link
Contributor

NeevCohen commented Mar 26, 2024

@sydney-runkle I opened a PR in core to support this - pydantic/pydantic-core#1249

When that will get merged I'll start working on a change in pydantic, which should be pretty straight forward

@NeevCohen
Copy link
Contributor

@sydney-runkle
Thanks for your help with the PR in core! Now that support was merged into core, I started looking into implementing the change in pydantic. I added a coerce_numbers_to_str option to FieldInfo, and I was wondering where would be the correct place to pass that value on to str_schema(). I thought maybe in GenerateSchema._common_field_schema, I could inject it into the schema variable? Not 100% sure on that, perhaps you have a better idea?

@sydney-runkle
Copy link
Member

sydney-runkle commented Mar 27, 2024

@NeevCohen,

Hmm, I think here would probably be the most appropriate location:

def apply_known_metadata(annotation: Any, schema: CoreSchema) -> CoreSchema | None: # noqa: C901

That function is used during the schema generation process :). You can also ensure that said setting is only specified for types with which it's compatible.

@NeevCohen
Copy link
Contributor

NeevCohen commented Mar 29, 2024

@sydney-runkle Thanks!

I've opened a PR #9137, but it can't be merged until a new version of core (with support for this) is released and the version is updated in pydantic.

@jkugler
Copy link
Author

jkugler commented Apr 11, 2024

Brilliant, thanks!

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

Successfully merging a pull request may close this issue.

3 participants