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

prevent int, float and Decimal from being converted to strings #150

Closed
samuelcolvin opened this issue Jul 6, 2022 · 12 comments · Fixed by #212
Closed

prevent int, float and Decimal from being converted to strings #150

samuelcolvin opened this issue Jul 6, 2022 · 12 comments · Fixed by #212

Comments

@samuelcolvin
Copy link
Member

This is a hangover from pydantic v1, should be removed.

@czotomo
Copy link
Contributor

czotomo commented Jul 6, 2022

Pardon me if this is a stupid question, but are we talking about not allowing these numerics in the lax_str Python parsing?

@samuelcolvin
Copy link
Member Author

That's what we're talking about.

See pydantic/pydantic#4218 and the preview for more context.

@czotomo
Copy link
Contributor

czotomo commented Jul 6, 2022

Aight, calling dibs on this one then

@samuelcolvin
Copy link
Member Author

samuelcolvin commented Jul 6, 2022

Let's wait a bit and see if there's strong pushback and whether we need a config setting.

Out of interest, @czotomo do you agree it's a good idea?

@czotomo
Copy link
Contributor

czotomo commented Jul 9, 2022

I do.
In this particular case there's simply way too much potential guessing and assuming to be made, so it makes sense to make a clear restriction from the get go.

@samuelcolvin
Copy link
Member Author

let's go for this, no one has responded negatively to this suggestion on the blog and I think it makes sense.

@samuelcolvin
Copy link
Member Author

samuelcolvin commented Jul 17, 2022

I'm afraid we are going to need some kind of compat config setting that makes this behaviour the same as pydantic v1 though.

Or maybe we don't, it's never going to be completely identical behaviour. @PrettyWood what do you think?

@PrettyWood
Copy link
Member

PrettyWood commented Jul 18, 2022

Hmm I would recommand to not do it unless a lot of people ask for it.
If it's only for some people, they can use a validator for this.

If it's really needed adding a allow_byte_string field with a default to false or even have an enum for strictness/coercion with 3 options instead of 2 for some types ('coercion': 'allow-string', 'default', 'none') could be a solution

@samuelcolvin
Copy link
Member Author

samuelcolvin commented Jul 19, 2022

@tiangolo what do you think about this, and about compatibility between v1 and v2 in general?

I understand that good compatibility overall would be good, but for stuff like this it would require significant work.

@samuelcolvin
Copy link
Member Author

I think we should go ahead with this.

@czotomo are you able to work on this? Otherwise, I'll do it when I get time.

@czotomo
Copy link
Contributor

czotomo commented Aug 2, 2022

@samuelcolvin I’m on vacations, I won’t have access to a computer until mid-August. If it can wait until then, I’d be glad to take it.

@tiangolo
Copy link
Member

tiangolo commented Aug 2, 2022

I think it should be fine to update this for V2. If there's a sort-of clear better way of doing something, I think that should be the one.

I know you're already trying to have as much compatibility as possible. I don't think there should be too much extra complexity if it's just for backwards compatibility with V1. That would also add maintenance burden, cruft, parameters and branches to test, and more things for users to learn, etc.

On the FastAPI side, I expect to add some extra glue code to try and be compatible with V1 and V2 for a while, to let people gradually migrate. Then they can pin Pydantic while they can migrate everything, but still being able to upgrade FastAPI versions. At least for a while.

There are also many uses and abuses (including mine) of different parts, and internal parts, that it's probably gonna be impossible to keep perfect compatibility with everything. I think it's a "rip the band-aid" sort of thing.

samuelcolvin added a commit that referenced this issue Aug 2, 2022
* prevent int,float,decimal -> str, fix #150

* fix tests

* coverage
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 a pull request may close this issue.

4 participants