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

BaseSettings validates default field value #166

Closed
stinovlas opened this issue Sep 13, 2023 · 9 comments · Fixed by #168
Closed

BaseSettings validates default field value #166

stinovlas opened this issue Sep 13, 2023 · 9 comments · Fixed by #168
Assignees

Comments

@stinovlas
Copy link
Contributor

Summary

BaseSettings, unlike BaseModel validates field default value. This contradicts the documentation. pydantic docs state:

Validators won't run when the default value is used. This applies both to @field_validator validators and Annotated validators. You can force them to run with Field(validate_default=True).

Documentation of pydantic-settings doesn't mention exception from this behavior.

I don't know whether this behavior is intended or not (I'd guess not, because workarounds are quite ugly, see below). But even if it is, it should probably be documented in pydantic-settings docs.

Minimal working example

from zoneinfo import ZoneInfo
from pydantic import BaseModel, ConfigDict
from pydantic.functional_validators import BeforeValidator
from pydantic_settings import BaseSettings, SettingsConfigDict
from typing_extensions import Annotated

class Model(BaseModel):
    model_config = ConfigDict(arbitrary_types_allowed=True)
    zone: Annotated[ZoneInfo, BeforeValidator(ZoneInfo)] = ZoneInfo("UTC")

class Settings(BaseSettings):
    # arbitrary_types_allowed are not needed for BaseSettings and don't change anything
    zone: Annotated[ZoneInfo, BeforeValidator(ZoneInfo)] = ZoneInfo("UTC")

print(repr(Model().zone))  # prints "zoneinfo.ZoneInfo(key='UTC')"
print(repr(Settings().zone))  # raises TypeError: expected str, bytes or os.PathLike object, not ZoneInfo

python -V

Python 3.11.2

pip --list

Package           Version
----------------- -------
annotated-types   0.5.0
pip               23.0.1
pydantic          2.3.0
pydantic_core     2.6.3
pydantic-settings 2.0.3
python-dotenv     1.0.0
setuptools        66.1.1
typing_extensions 4.7.1

Workarounds

  1. Use plain default value. This works as expected, but doesn't play well with type checkers:
class Settings(BaseSettings):
    zone: Annotated[ZoneInfo, BeforeValidator(ZoneInfo)] = "UTC"
  1. Set validate_default=False explicitely. This works, but is unnecessarily verbose:
class Settings(BaseSettings):
    zone: Annotated[ZoneInfo, Field(validate_default=False), BeforeValidator(ZoneInfo)] = ZoneInfo("UTC")
@hramezani
Copy link
Member

Thanks @stinovlas for reporting this 🙏

You are right. pydantic-settings validates default value and we need to document it.

Would you like to open a PR?

@stinovlas
Copy link
Contributor Author

Would you like to open a PR?

Before I do open anything, I'd like to know a bit more about reasoning. Why does pydantic-settings do that, opposed to pydantic default? I guess that disabling it is as simple as passing model_config = SettingsConfigDict(validate_default=False) to the Settings model, but I'm still not clear why is this default? And if it's intentional, how do you propose to cope with the static checkers problems I mentioned?

@hramezani
Copy link
Member

I'd like to know a bit more about reasoning. Why does pydantic-settings do that, opposed to pydantic default?

It is the same behavior in settings management in Pydantic V1. not sure what is the reason but I think it is the right behavior. and we want to keep the behavior as much as possible unchanged.

I guess that disabling it is as simple as passing model_config = SettingsConfigDict(validate_default=False)

Yes

And if it's intentional, how do you propose to cope with the static checkers problems I mentioned?

Why did you annotateZoneInfo with BeforeValidator(ZoneInfo)? what do you want to achieve?

@stinovlas
Copy link
Contributor Author

And if it's intentional, how do you propose to cope with the static checkers problems I mentioned?

Why did you annotateZoneInfo with BeforeValidator(ZoneInfo)? what do you want to achieve?

I load most of my settings either from env variables or static config files (usually YAML, but format does not matter). You can't represent ZoneInfo type in those formats. I chose ZoneInfo because it's a simple type from demonstration and is present in standard library, but I have this problem with many different non-trivial types. I need to take a string representation from the env variable and turn it into complex type. In this case, ZoneInfo is a callable that accepts string and returns ZoneInfo instance. So I can do something like this:

ZONE=Europe/Prague python example.py

Maybe this is not the recommended way to load strings into more complex types? If you know about a better pattern, I'll be happy to learn about it.

Anyway, I think that I know enough to create PR for documentation update. I'll put it together and submit a PR. Pydantic is fantastic library and I'm happy I'll be able to contribute my tiny bit to it 🙂.

stinovlas added a commit to stinovlas/pydantic-settings that referenced this issue Sep 15, 2023
stinovlas added a commit to stinovlas/pydantic-settings that referenced this issue Sep 15, 2023
@hramezani
Copy link
Member

hramezani commented Sep 15, 2023

You can use the following code:

from zoneinfo import ZoneInfo
from pydantic import BeforeValidator
from pydantic.functional_validators import BeforeValidator
from pydantic_settings import BaseSettings
from typing_extensions import Annotated

class Settings(BaseSettings):
    zone: Annotated[ZoneInfo, BeforeValidator(lambda x: ZoneInfo(x) if isinstance(x, str) else x)] = ZoneInfo("UTC")

print(repr(Settings().zone))

stinovlas added a commit to stinovlas/pydantic-settings that referenced this issue Sep 15, 2023
stinovlas added a commit to stinovlas/pydantic-settings that referenced this issue Sep 15, 2023
@stinovlas
Copy link
Contributor Author

You can use the following code:

from zoneinfo import ZoneInfo
from pydantic import BeforeValidator
from pydantic.functional_validators import BeforeValidator
from pydantic_settings import BaseSettings
from typing_extensions import Annotated

class Settings(BaseSettings):
    zone: Annotated[ZoneInfo, BeforeValidator(lambda x: ZoneInfo(x) if isinstance(x, str) else x)] = ZoneInfo("UTC")

print(repr(Settings().zone))

Well of course I can :-). In fact, if I were doing something like this, I'd probably enclose it to a separate function. Or used the @field_validator. Or I could write a decorator to convert any str-accepting type constructor to do the same. I was just looking for the solution that is clean and does not add unnecessary wrapper code.

I opened #168 with the docs amend ;-).

@hramezani
Copy link
Member

‌I was just looking for the solution that is clean and does not add unnecessary wrapper code

what do you mean by clean? you want to convert the string to a ZoneInfo object and you need to do the coversion.

I opened #168 with the docs amend ;-).

Thanks! I will take a look!

@stinovlas
Copy link
Contributor Author

‌I was just looking for the solution that is clean and does not add unnecessary wrapper code

what do you mean by clean? you want to convert the string to a ZoneInfo object and you need to do the coversion.

This is highly opinionated and I certainly don't want to force my opinions on anyone. So, just for the illustration of what I consider clean (this is a bit off-topic, but I intend to not to dive further):

Clean code is easy to read

After all, code is usually written once, read many times. For this reason, I like the Annotated syntax for declaring model fields over using @field_validator. There's nothing wrong with @field_validator, but it is decoupled from the field itself, which makes it a bit harder to read. Annotated syntax crates longer field definitions, but you have a clear idea what the applied validators are, just by looking at it.

Writing lambda to Annotated field is of course possible, but from my point of view, it makes the annotation harder to read. Especially, it can create long and convoluted lines. Proper function with explanatory name would be easier to read.

Clean code does not repeat itself

But, if I need to create helper function for each data type, it can get a bit messy. Of course, I could (and would) enclose those helper functions to separate module, but maybe I could even build a decorator that takes a type and returns a function that applies the type on its argument unless it already is an instance of that type. That would simplify things down. But...

Simple is better than complex

Such decorator can be a bit hard to understand, especially for less experienced developers who may need to work on the code base.

I have to give you one thing. Explicit conversion function is probably better than just using the type as a conversion tool. This validator has the undeniable advantage of being source-agnostic.

I can imagine pydantic doing this conversion auto-magically, but I can also see valid arguments against such behaviour (mostly explicit is better than implicit).

@hramezani
Copy link
Member

Thanks for explaning.

Writing lambda to Annotated field is of course possible, but from my point of view, it makes the annotation harder to read. Especially, it can create long and convoluted lines. Proper function with explanatory name would be easier to read.

The lambda was an example to make it short. for sure you can define your own conversion function and use it.

But, if I need to create helper function for each data type, it can get a bit messy. Of course, I could (and would) enclose those helper functions to separate module, but maybe I could even build a decorator that takes a type and returns a function that applies the type on its argument unless it already is an instance of that type. That would simplify things down. But...

You are using a custom data type which Pydantic does not support it internally. If you have different approach of conversion you need to write different conversion functions and you are not repeating yourself.

In general I would say IMHO, there shouldn't be a big difference between BeforeValidator(ZoneInfo) and BeforeValidator(<conversion_function>) if the conversion_function name and code be clear.

hramezani added a commit that referenced this issue Sep 18, 2023
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.

2 participants