pydantic / pydantic

Data validation using Python type hints
https://docs.pydantic.dev
MIT License
20.46k stars 1.84k forks source link

Default of `allow_inf_nan` for `Decimal` and `float` are inconsistent #7296

Open davidhewitt opened 1 year ago

davidhewitt commented 1 year ago

Initial Checks

Description

Decimal validation doesn't allow nan or infinity by default. float validation does allow these values by default.

We should consider making these defaults the same (possibly a V3 thing).

Example Code

from pydantic import TypeAdapter
from decimal import Decimal

TypeAdapter(float).validate_python(float("nan"))  # ok
TypeAdapter(Decimal).validate_python(float("nan"))  # raises ValidationError

Python, Pydantic & OS Version

pydantic version: 2.3.0
        pydantic-core version: 2.6.3
          pydantic-core build: profile=release pgo=true
                 install path: /home/david/.virtualenvs/pydantic-prod/lib/python3.11/site-packages/pydantic
               python version: 3.11.4 (main, Jun  9 2023, 07:59:55) [GCC 12.3.0]
                     platform: Linux-5.15.90.1-microsoft-standard-WSL2-x86_64-with-glibc2.37
     optional deps. installed: ['email-validator', 'typing-extensions']

Selected Assignee: @adriangb

fatelei commented 1 year ago

float schema define

image

decimal schema define

image

maybe add a tip in document

fatelei commented 1 year ago

update pydantic core

let allow_inf_nan = schema_or_config_same(schema, config, intern!(py, "allow_inf_nan"))?.unwrap_or(false);

to

let allow_inf_nan = schema_or_config_same(schema, config, intern!(py, "allow_inf_nan"))?.unwrap_or(true);
behrenhoff commented 10 months ago

Just a question: Considering this code:

class Foo(pydantic.BaseModel):
    foo: Annotated[float, pydantic.Field(gt=0)]

Foo(foo=np.nan)

Validates even though I have asked for >0. nan clearly isn't greater than 0, my expectation was that it compares false in all cases. Should this be considered a bug or am I missing some documentation? I am trying to find out what allow_inf_nan=_Unset (the default) actually means. My expectation was: no nans allowed by default when any comparison like gt, ge, lt, ... is set. Apparently that's wrong. It is also different from pydantic v1 where a Field(gt=0) would not allow nan and I didn't find it in mentioned in the migration guide.

davidhewitt commented 10 months ago

@behrenhoff I agree that's a bug, opened https://github.com/pydantic/pydantic-core/pull/1037 to address.

davidhewitt commented 9 months ago

From https://github.com/pydantic/pydantic-core/pull/1037#issuecomment-1855434448

Mathematically, this PR doesn't make sense: With constraints, I define the range my valid numbers have to be in. Nans don't represent numbers, they represent missing/incomputable data. So, nans have to be excluded from constraints.

Hence, when I explicitly define a Field to accepts nans because I expect missing data, I should be able to define a range all other data has to be in.

I agree that allow_inf_nan should be explicitly set to True and that it should validate the same for floats and Decimals.

@sasanjac - Maybe someone wants to allow positive infinity but not NaN, then allow_inf_nan=True and gt=0 would currently fit that use case, but not your use case.

I wonder if this is motivation to split allow_inf and allow_nan into separate flags in V3? Perhaps the default of allow_inf can be True, and allow_nan could be "True unless a constraint is applied", though that's a bit complex.

You could define your own AfterValidator for now which does the range checking and accepts NaN.

sasanjac commented 9 months ago

I agree. Splitting the flags would improve usability a lot. However, since nans can not ever conform to numerical constraints they don't matter when defining them: Either I expect missing/false data or I don't, regardless of constraints on my numerical data. So, I'd support a simple approach and either set allow_nan to True by default and let the user explicitly disallow it or the other way around