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

Type signature for Field() gt/lt/gte/lte validators should include timedelta #9472

Closed
1 task done
bjmc opened this issue May 21, 2024 · 7 comments · Fixed by #9484
Closed
1 task done

Type signature for Field() gt/lt/gte/lte validators should include timedelta #9472

bjmc opened this issue May 21, 2024 · 7 comments · Fixed by #9484
Labels
bug V2 Bug related to Pydantic V2 pending Awaiting a response / confirmation

Comments

@bjmc
Copy link
Contributor

bjmc commented May 21, 2024

Initial Checks

  • I confirm that I'm using Pydantic V2

Description

The type signature for the numeric min/max value constraints on Field() (i.e. gt, lt, gte, lte ) specified here is too restrictive, allowing only float or None.

In actual use, the validation works correctly as expected with timedelta() instances, but static type checkers (at least Pyright) object.

Example Code

from datetime import timedelta

from pydantic import BaseModel, Field

class Duration(BaseModel):
    td: timedelta = Field(timedelta(0), gt=timedelta(0))


Duration(td=timedelta(seconds=1))
Duration(td=timedelta(seconds=-1))
# ValidationError: 1 validation error for Duration
# td
#  Input should be greater than 0 seconds [type=greater_than, input_value=datetime.timedelta(days=-1), input_type=timedelta]
#    For further information visit https://errors.pydantic.dev/2.7/v/greater_than

# This is all perfectly valid code, but Pyright (and I assume also mypy) complains about the type signature

# $ pyright example.py 
# /home/bjmc/Sandbox/example.py
#  /home/bjmc/Sandbox/example.py:6:44 - error: Argument of type "timedelta" cannot be assigned to # parameter "gt" of type "float | None" in function "Field"
#    Type "timedelta" is incompatible with type "float | None"
#      "timedelta" is incompatible with "float"
#      "timedelta" is incompatible with "None" (reportArgumentType)
# 1 error, 0 warnings, 0 informations

Python, Pydantic & OS Version

pydantic version: 2.7.1
        pydantic-core version: 2.18.2
          pydantic-core build: profile=release pgo=true
                 install path: /home/bjmc/.cache/pypoetry/virtualenvs/ska-oso-pdm-zPbyUnCb-py3.10/lib/python3.10/site-packages/pydantic
               python version: 3.10.12 (main, Nov 20 2023, 15:14:05) [GCC 11.4.0]
                     platform: Linux-6.5.0-1022-oem-x86_64-with-glibc2.35
             related packages: typing_extensions-4.11.0 pyright-1.1.363
                       commit: unknown
@bjmc bjmc added bug V2 Bug related to Pydantic V2 pending Awaiting a response / confirmation labels May 21, 2024
@bjmc
Copy link
Contributor Author

bjmc commented May 21, 2024

I'm not a typing wizard, but I wonder if a good resolution here would be to use the comparison protocols defined by typeshed for these field validators?

I'm happy to open an PR if people like that approach, or have another suggestion.

@sydney-runkle
Copy link
Member

@bjmc,

Hmm, I'm intrigued - feel free to open a PR and I can review. I don think it could be good to offer more general support here!

@Viicos
Copy link
Member

Viicos commented May 21, 2024

Seems reasonable to use the comparison protocols, internally Pydantic uses the functions defined in pydantic._internal._validators (e.g. greater_than_validator), so this seems like a safe improvement.

Two options here:

  • Use the protocols defined in annotated_types (best option imo).
  • Import the protocols from _typeshed in an if TYPE_CHECKING: block. They are not defined as being stable.

@sydney-runkle
Copy link
Member

Let's go with the annotated_types protocols

@bjmc
Copy link
Contributor Author

bjmc commented May 22, 2024

I agree that annotated_types is better since it's already a dependency.

@bjmc
Copy link
Contributor Author

bjmc commented May 22, 2024

@sydney-runkle I've got a draft PR up. I'm not quite sure where to add a test for this, since it's only a type-checking issue. Suggestions?

@sydney-runkle
Copy link
Member

sydney-runkle commented May 28, 2024

@bjmc,

Perhaps you could just add a before / after example to this PR running pyright on a file that uses a timedelta gt constraint as a proof of concept? PR looks great to me!

Update: I've added some notes to your PR re testing + some change requests!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug V2 Bug related to Pydantic V2 pending Awaiting a response / confirmation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants