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

Can no longer parse str() of timedelta #8248

Open
1 task done
brasie opened this issue Nov 29, 2023 · 5 comments · May be fixed by pydantic/speedate#74
Open
1 task done

Can no longer parse str() of timedelta #8248

brasie opened this issue Nov 29, 2023 · 5 comments · May be fixed by pydantic/speedate#74
Assignees
Labels
feature request help wanted Pull Request welcome

Comments

@brasie
Copy link

brasie commented Nov 29, 2023

Initial Checks

  • I confirm that I'm using Pydantic V2

Description

In V1, one could parse the a str()ed timedelta:

parse_obj_as(timedelta, str(timedelta(hours=3)))

But the equivalent in V2 breaks:

TypeAdapter(timedelta).validate_python(str(timedelta(hours=3)))

This seems simply because the V2 parser is more strict about a leading zero in the hours part of the timedelta, i.e. 3:00:00 is rejected (as output by timedelta.str), but 03:00:00 is ok.

It would be nice to preserve the ability to parse the result of timedelta.str

Example Code

import datetime

import pydantic

class Foo(pydantic.BaseModel):
    td: datetime.timedelta

# Works
Foo.model_validate({"td": str(datetime.timedelta(hours=10))})
Foo.model_validate({"td": str(datetime.timedelta(days=3, hours=10))})
Foo.model_validate({"td": str(datetime.timedelta(days=3, hours=10, minutes=5, seconds=10))})

# Fails
Foo.model_validate({"td": str(datetime.timedelta(days=3, hours=9, minutes=5, seconds=10))})

Python, Pydantic & OS Version

pydantic version: 2.5.2
        pydantic-core version: 2.14.5
          pydantic-core build: profile=release pgo=true
                 install path: /.../pypoetry/virtualenvs/...-ldZDDfYB-py3.11/lib/python3.11/site-packages/pydantic
               python version: 3.11.5 (main, Sep  2 2023, 14:16:33) [GCC 13.2.1 20230801]
                     platform: Linux-6.5.9-arch2-1-x86_64-with-glibc2.38
             related packages: mypy-1.7.1 typing_extensions-4.8.0
@brasie brasie added bug V2 Bug related to Pydantic V2 pending Awaiting a response / confirmation labels Nov 29, 2023
@sydney-runkle
Copy link
Member

Hi @brasie,

Thanks for reporting this. Pydantic now uses speedate for date/time related parsing.

The Pydantic v1 -> v2 migration guide provides this table with information about how Pydantic converts data during validation.

The table also illustrates that when you pass a str in for a timedelta field, it's assumed to be in ISO8601 format. Taking the str of a datetime.timedelta doesn't necessarily conform to that, as seen in your example.

If you still want to be able to validate data in this format, I'd recommend a custom field validator. Here's an example I threw together quite quickly, there might be some oversights here re behavior with days / without days included in the timedelta:

import datetime
import json

from pydantic import BaseModel, field_validator


class Foo(BaseModel):
    td: datetime.timedelta

    @field_validator('td', mode='before')
    def td_to_timedelta(cls, v):
        if isinstance(v, datetime.timedelta):
            return v
        elif isinstance(v, str):
            dt = datetime.datetime.strptime(v, '%d days, %H:%M:%S') if 'days' in v else datetime.datetime.strptime(v, '%H:%M:%S')
            return datetime.timedelta(days=dt.day, hours=dt.hour, minutes=dt.minute, seconds=dt.second)
        else:
            return v


# Works
Foo.model_validate({'td': str(datetime.timedelta(hours=10))})
Foo.model_validate({'td': str(datetime.timedelta(days=3, hours=10))})
Foo.model_validate({'td': str(datetime.timedelta(days=3, hours=10, minutes=5, seconds=10))})
Foo.model_validate_json(json.dumps({'td': str(datetime.timedelta(days=3, hours=9, minutes=5, seconds=10))}))

An even cleaner approach would be to just pad the hours if a single digit it used.

Closing for now, as this is more of a question than a bug. Thanks!

@sydney-runkle sydney-runkle self-assigned this Nov 29, 2023
@sydney-runkle sydney-runkle added question and removed bug V2 Bug related to Pydantic V2 pending Awaiting a response / confirmation labels Nov 29, 2023
@brasie
Copy link
Author

brasie commented Nov 29, 2023

I see, I suppose; Thanks for the explanation.

It's just, you're already parsing more than ISO8601, assuming my understanding is correct that ISO8601 does not include a datetime.timedelta.__str__-like [D days,] HH:MM:SS format. Pydantic V1 also supported reading a timedelta.__str__-like format, but unlike Pydantic V1, V2 no longer supports all ways that timedelta.__str__ outputs.

Hence why it seemed bug-like.

But if it is expected behavior on your side, then perhaps this is more of a feature request for speedate.

@samuelcolvin
Copy link
Member

I think it makes sense to support all timedelta str representations in speedate.

@samuelcolvin samuelcolvin reopened this Nov 29, 2023
@sydney-runkle sydney-runkle added the help wanted Pull Request welcome label Nov 30, 2023
@rafrafek
Copy link

rafrafek commented Nov 30, 2023

In my case v1 serialised datetime.timedelta to:

"session_expiry": 10800,

and v2 serialises it to:

"session_expiry": "PT10800S",

I fixed it by:

@field_serializer("session_expiry", mode="plain")
def session_expiry_to_seconds(self, session_expiry: datetime.timedelta) -> int:
    return session_expiry.seconds

Edit:
There is a bug in my code above, check the comment bellow. @igoose1, thanks for spotting it!

@igoose1
Copy link

igoose1 commented Jan 16, 2024

@rafrafek, thanks! I would notice that your solution can give an unexpected answer as timedelta.seconds are always less than 1 day and don't include microseconds. You probably want to use .total_seconds() instead.

Example:

>>> td = datetime.timedelta(days=4, seconds=20, microseconds=69)
>>> td.seconds
20
>>> td.total_seconds()
345620.000069

So this code can be re-written in this to avoid issues:

@field_serializer("session_expiry", mode="plain")
def session_expiry_to_seconds(self, session_expiry: datetime.timedelta) -> float:  # now it returns a float
    return session_expiry.total_seconds()  # now it returns all seconds

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request help wanted Pull Request welcome
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants