Giter VIP home page Giter VIP logo

Comments (12)

Viicos avatar Viicos commented on May 27, 2024 1

You are looking for https://docs.pydantic.dev/latest/api/config/#pydantic.config.ConfigDict.validate_default

from pydantic.

RajatRajdeep avatar RajatRajdeep commented on May 27, 2024

Sharing one more reference from the doc: https://docs.pydantic.dev/2.7/concepts/fields/#validate-default-values

from pydantic.

JohnHind avatar JohnHind commented on May 27, 2024

Thanks guys, but this ought to work?

from pydantic import BaseModel, ConfigDict, with_config
from pydantic.version import version_info

@with_config(ConfigDict(validate_default=True))
class Command(BaseModel):
    chan: int = 0,
    val: int = 0

validated = Command(val = 3)

print(validated.chan, type(validated.chan), validated.val, type(validated.val))
print(version_info())

No change from before, no warning that I've done anything wrong.

from pydantic import BaseModel, ConfigDict
from pydantic.version import version_info

class Command(BaseModel):
    model_config = ConfigDict(validate_default=True)
    chan: int = 0,
    val: int = 0

validated = Command(val = 3)

print(validated.chan, type(validated.chan), validated.val, type(validated.val))
print(version_info())

This works.

from pydantic import BaseModel, ConfigDict
from pydantic.version import version_info

class MyBaseModel(BaseModel):
    model_config = ConfigDict(validate_default=True)

class Command(MyBaseModel):
    chan: int = 0,
    val: int = 0

validated = Command(val = 3)

print(validated.chan, type(validated.chan), validated.val, type(validated.val))
print(version_info())

This also works and seems cleaner, particularly if there is more than one schema to modify.

But I have to say I'm not impressed. This behavior ought to be the default, with the ability to switch it off, and the documentation should be a lot clearer. Also the validation error message is not very clear. It does not mention that it is validation of a default that has failed, or the name of the specific defaulting field.

from pydantic.

Viicos avatar Viicos commented on May 27, 2024

No change from before, no warning that I've done anything wrong.

with_config was added explicitly for dataclasses and TypedDict (see the docstring and the related usage docs. However, I agree we should either:

  • Support using the decorator on Pydantic models (not sure this is easily doable)
  • raise a runtime error if the decorator is used on a Pydantic model

I'll open an issue to work on this.

This behavior ought to be the default, with the ability to switch it off

I don't know if there was a specific reason to have them not validated by default, but this can't be changed now for backwards compatibility reasons.

the documentation should be a lot clearer.

What part of the documentation do you find unclear? It does state the current behavior.

Also the validation error message is not very clear. It does not mention that it is validation of a default that has failed, or the name of the specific defaulting field.

Seems like a reasonable FR.

from pydantic.

JohnHind avatar JohnHind commented on May 27, 2024

I don't know if there was a specific reason to have them not validated by default, but this can't be changed now for backwards compatibility reasons.

What part of the documentation do you find unclear?

Look at "A Simple Example" in the Github readme: this has defaults in the decorator and the 'name' field is actually defaulted in the validation. There is no indication that this is not validated, or how to fix it. This problem percolates right through the ecosystem in many starter tutorials, YouTube videos and documentation for other libraries and frameworks which use Pydantic. I came to it through FastAPI and only was aware because I made a typo in my code. Otherwise I had no reason to 'go down this rabbit hole'.

I suggest the following: Ship and promote a NewBaseModel which takes a 'safety first' approach to all validation, setting the defaults on this basis regardless of compatibility, but continue to ship the current BaseModel so existing code is not broken. This could be done using the inheritance method I've used above, but this is not ideal since assigning model_config could unexpectedly disable validation again. Ideally this should be implemented 'under the hood' so the defaults with no model_config are changed.

Seems like a reasonable FR.

I'll raise it! (Improving the validation error message)

Thanks for your answers on the other aspects of this matter which seems like a good way forward.

from pydantic.

Viicos avatar Viicos commented on May 27, 2024

It honestly takes 5 minutes of reading in the Concepts docs page (which people usually read first when discovering Pydantic) to come across the section about opt-in validation of default values. Maybe a quick note can be added here, but I don't see what more can be done.

I'll also note that a type checker would have immediately caught the typo:

image

This may be why validate_default does not default to True, as most of the time (unless maybe when using default_factory) the error is easily reported by a type checker.


I suggest the following: Ship and promote a NewBaseModel which takes a 'safety first' approach to all validation

You can open a new issue for this to catch attention from maintainers, but there's a high chance it will be rejected, as it is overly specific

from pydantic.

JohnHind avatar JohnHind commented on May 27, 2024

You can open a new issue for this to catch attention from maintainers, but there's a high chance it will be rejected, as it is overly specific

Yea, typical coder Catch-22 response! Initial response: "Can't fix that, it would break existing code". I suggest a way of doing it that does not break existing code. "Won't do that it is overly specific." 'Overly specific' = 'not invented here'!

from pydantic.

Viicos avatar Viicos commented on May 27, 2024

Well yeah introducing a NewBaseModel just to tweak a configuration value is not a viable solution when you can use a two lines workaround as you proposed 1.

Looking at the previous duplicate issues, it seems to be disabled by default because of performance reasons. I'm wondering if with the v2 performance improvements this would still be an issue.


While I agree the default behavior of not validating default values is debatable, I don't think introducing a NewBaseModel is the good way to go: what if someone requests the default value of another setting (say extra, currently defaulting to "ignore") to change; should we create another base model as well?

Maybe this could be considered as a breaking change in V3 and the default value should simply change to True, but considering:

  • type checkers can easily catch the issue
  • there's a simple workaround by defining a custom base model with your configuration values,

I unfortunately don't think this will be accepted by maintainers

Footnotes

  1. Regarding your concern about overwriting model_config (This could be done using the inheritance method I've used above, but this is not ideal since assigning model_config could unexpectedly disable validation again.): Pydantic supports merging configurations together.

from pydantic.

JohnHind avatar JohnHind commented on May 27, 2024
  1. Regarding your concern about overwriting model_config (This could be done using the inheritance method I've used above, but this is not ideal since assigning model_config could unexpectedly disable validation again.): Pydantic supports merging configurations together.

OK, I'd not realized this would result in both configuration dictionaries being applied (presumably in base-first order), nor that any old dict would do, it did not actually have to be a ConfigDict! Seems like a very obscurantist way of doing this though, I'd have had a config method which took VarArgs, then it would be clear the settings were 'applied' not just stored for direct use as implied by making them a property.

BUT this completely undermines any objection to Pydantic taking this approach! If it is only two lines for me, it is only two lines for them too! What I was trying to achieve is to make the simple beginners example work out of the box as a new user would reasonably expect without making it any more complex or breaking existing code. I am a big believer in the approach of minimizing documentation by designing more intuitive software that behaves the way you expect out of the box. If I have to read all the documentation for all the dependencies of every library I use it completely undermines the case for using libraries at all, it would be quicker just to write the bits I need myself!

from pydantic.

Viicos avatar Viicos commented on May 27, 2024

I'd have had a config method which took VarArgs, then it would be clear the settings were 'applied' not just stored for direct use as implied by making them a property.

What is the API you have in mind? I'm not sure I understand a config method which took VarArgs, but I'm interested to know :)

Seems like a very obscurantist way of doing this though

Pydantic is doing a lot internally to handle config, and it does not only rely on a simple dict (it uses a class, ConfigWrapper). Imo having a simple ConfigDict TypedDict as a public interface is great, you don't have to worry about internal details, but I do agree it can be confusing, mainly because:

  • the config dict is currently following the structure config_key -> config_value, but what if we ever get a nested config attribute?
class Base(BaseModel):
    model_config = {"nested_config": {"attr1": 1}}

class Sub(Base):
    model_config = {"nested_config": {"attr2": 2}}

# How is it merged? Is Sub.model_config ==
# - {"nested_config": {"attr1": 1}}
# - {"nested_config": {"attr2": 2}}
# - {"nested_config": {"attr1": 1, "attr2: 2}}?
  • There's currently no API to inspect the "merged" configuration of a model, i.e.:
class Base(BaseModel):
    model_config = {"validate_default": True}

class Sub(Base):
    model_config = {"extra": "forbid"}

Sub.model_config["validate_default"]
#> True? False? KeyError?
  • The merging behavior doesn't seem to be documented yet?

If it is only two lines for me, it is only two lines for them too!

It's not really about how much work it requires to implement it in Pydantic, my main concern is the third paragraph from my previous comment

@sydney-runkle, what is your opinion on this? How much of a breaking change would it be validate defaults by default? (assuming the performance issues are now resolved).

from pydantic.

JohnHind avatar JohnHind commented on May 27, 2024

What is the API you have in mind? I'm not sure I understand a config method which took VarArgs, but I'm interested to know :)

Yea, I didn't really think that through! If it was a method, where'd it get called? I guess maybe this is the best way, though as you say, it does need to be better documented, and a diagnostic function to interrogate the effective setting would also be useful.

It's not really about how much work it requires to implement it in Pydantic, my main concern is the third paragraph from my previous comment

I see the danger of proliferation, but I was thinking about it as a one-off policy change: It would be made clear that the new base might implement additional checks in future so you'd better not try to exploit any loopholes. But if you need stability or maximum performance continue to use BaseModel and we promise that anything new will be off by default.

from pydantic.

sydney-runkle avatar sydney-runkle commented on May 27, 2024

@sydney-runkle, what is your opinion on this? How much of a breaking change would it be validate defaults by default? (assuming the #669 (comment) are now resolved).

Yeah, I think this would be a significant breaking change, definitely something we couldn't consider until at least V3. We thought through this feature pretty carefully for V2, see https://docs.pydantic.dev/latest/migration/#changes-to-validators.

Thanks @Viicos for that new PR for the with_config fix, looking now!

from pydantic.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.