✨ Add JSON-compatible float constraints for NaN and Inf

[tiangolo]

Change Summary

✨ Add JSON-compatible float constraints for NaN and Inf (+inf, -inf).

Full discussion in FastAPI: fastapi/fastapi#4589

Short example

from typing import List

from fastapi import FastAPI
from pydantic import BaseModel

app = FastAPI()


class DataModel(BaseModel):
    a_string_value: str
    a_float_value: float


stored_objects: List[DataModel] = []


@app.get("/", response_model=List[DataModel])
async def get_objects():
    return stored_objects


@app.post("/")
async def save_object(to_save: DataModel):
    return stored_objects.append(to_save)

Then:

$ curl localhost:8000 -H 'Content-Type: application/json' -d '{"a_string_value": "object 1", "a_float_value": "NaN"}'
$ curl localhost:8000

That creates an InternalServerError with ValueError: Out of range float values are not JSON compliant.

Reason

Pydantic parses for a float field any string that could be parsed by Python's float, including "NaN". But Starlette encodes JSON responses with json.dumps(allow_nan=False). This means that Pydantic will "successfully" parse the "NaN" string as a Python float (which is not valid JSON but is valid Python and valid JavaScript), but then the response will not be able to convert the values back to JSON.

Alternatives

Change the response in Starlette/FastAPI

Changing the behavior in the response in Starlette (or FastAPI) to use json.dumps(allow_nan=True) (the default) would convert those float("nan") values to JSON null, the equivalent of Python None.

But that would mean that any type annotated as float would implicitly actually be Union[float, None] when converted to JSON. But that would be counterintuitive as static analysis (mypy, the editor, autocompletion, inline errors, TypeScript automatically generated clients, etc) would expect a float to be just float, not that in a corner case it could be None/null.

Accept this PR and have the new Pydantic types

If this PR is accepted, then my plan would be to document in FastAPI that using float in Python has those caveats, and it might be better to use the new Pydantic's JSONFloat that ensures compatibility with JSON.

Change the default behavior of float

Currently, there are no tests about how float('NaN'), float('+inf'), and float('-inf') should behave (although there's the current implicit behavior of accepting them).

There could be the option to require floats to not be NaN, +inf, or -inf, the same way that currently Decimals are treated. Currently, there are even tests that check that Decimal('NaN') is disallowed. So it might not be crazy to replicate the same behavior for floats.

Related issue number

Related to this issue: #1779 in particular to this comment: #1779 (comment)

I wouldn't say this directly closes that issue, but it's indeed related.

It is also related to this FastAPI issue: fastapi/fastapi#4589

Next

I want to gauge the interest/acceptability of this or the alternatives before writing the docs.

Checklist

• Unit tests for the changes exist
• Tests pass on CI and coverage remains at 100%
• Documentation reflects the changes where applicable
• changes/<pull request or issue id>-<github username>.md file added describing change
  (see changes/README.md for details)
• My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

[tiangolo]

please review

[samuelcolvin]

Otherwise I'm happy with this idea, needs some docs.

pydantic/pydantic-core#224 to remind us we need to update core to support this.

[samuelcolvin]

My only question is: do we need both of these options?

I can't imagine a scenario where we want to allow inf but not nan or visa. versa.???

I would say we should have one allow_nan_inf argument for clarity.

@tiangolo what do you think?

[samuelcolvin]

please update.

[samuelcolvin]

To match pydantic/pydantic-core#228, we need one option allow_nan_inf, default to True.

[samuelcolvin]

@tiangolo you get special treatment, I'll try to fix this for you today.

---

Notice

See twitter 🐦, I've you'd like this to be included in V1.10, please fix it and request a review TODAY.

Or if you need this in V1.10 but don't have time to complete it (or aren't the author), please comment here and on #4324.

[samuelcolvin]

should be mostly there, just needs some docs.

@tiangolo @PrettyWood please review.

[PrettyWood]

LGTM! Should we add also this option at config level? I feel like it's something you often want enabled or disabled for the whole model

[samuelcolvin]

Good point, I'll do that now. It'll also match V2.

[PrettyWood]

Great

[PrettyWood]

👍

[tiangolo]

Amazing! Thanks for taking over it @samuelcolvin! 🙇 LGTM 🚀

[samuelcolvin]

thanks again for this.