glyph / DateType

A type wrapper for the standard library `datetime` that supplies stricter checks, such as making 'datetime' not substitutable for 'date', and separating out Naive and Aware datetimes into separate, mutually-incompatible types.
Other
73 stars 6 forks source link

Date/DateTime + relativedelta() #11

Open SonOfLilit opened 1 month ago

SonOfLilit commented 1 month ago

Hi,

You're my hero. Thanks.

Also, I want to actually use your library in my code, but I'm getting errors for my_date + relativedelta(days=1) (and same with datetimes). I wonder what it would take to fix it.

Can your pyi add additional overloads to [https://github.com/python/typeshed/blob/main/stubs/python-dateutil/dateutil/relativedelta.pyi](typeshed's relativedelta.pyi)?

class relativedelta:
    # ...
    @overload
    def __add__(self, other: relativedelta) -> Self: ...
    @overload
    def __add__(self, other: timedelta) -> Self: ...
    @overload
    def __add__(self, other: _DateT) -> _DateT: ...
    # and __radd__ etc'

I guess we can add a fake Date.__add__(self, other: relativedelta) instead and it would do the right thing? Do we need to entirely replace typeshed's relativedelta for this to work?

How can I approach getting this to work? Will you accept a PR? Is there a workaround I can use in the meantime?

You should be aware of this before you answer, I guess. One of the weirder corners of Python:

>>> from dateutil.relativedelta import relativedelta
>>> from datetime import date, datetime
>>> date(1970, 1, 1) + relativedelta(days=1)
datetime.date(1970, 1, 2)
>>> date(1970, 1, 1) + relativedelta(minutes=1)
datetime.datetime(1970, 1, 1, 0, 1)

I guess we could have something like:

    @overload
    def __add__(self, other: Date) -> Date | DateTime: ...
SonOfLilit commented 1 month ago

Here's what a PR would look like it we didn't care about people who don't have dateutil installed:

https://github.com/SonOfLilit/DateType/commit/7690b5b347273236596971edac15add6fa1693b2

glyph commented 1 month ago

@SonOfLilit I think we could do this, if you protected the import with an if TYPE_CHECKING and we made sure to have 2 CI jobs, one with relativedelta and one without.

cc @pganssle will I regret this approach?

glyph commented 1 month ago

I kinda hate the Self | DateTime, but I realize we wouldn't want to do this without it since it would be incorrect to assert you can only get dates. I feel like a RelativeDateDelta and a RelativeDateTimeDelta would want to be two different types in the DateType cinematic universe, but perhaps the patch could be to dateutil instead?

SonOfLilit commented 1 month ago

I'm afraid dateutil are not much more likely to accept our patches than the standard library, so we will need to create a dateutiltype?

glyph commented 1 month ago

I'm afraid dateutil are not much more likely to accept our patches than the standard library

Why not?

I'm not entirely opposed to landing the changes here, assuming we have at least tacit buy-in from the dateutil maintainers

SonOfLilit commented 1 month ago

I'll ask

On Wed, Aug 7, 2024, 09:43 Glyph @.***> wrote:

I'm afraid dateutil are not much more likely to accept our patches than the standard library

Why not?

I'm not entirely opposed to landing the changes here, assuming we have at least tacit buy-in from the dateutil maintainers

— Reply to this email directly, view it on GitHub https://github.com/glyph/DateType/issues/11#issuecomment-2272736504, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAADU7JQQ37HVESVYABGML3ZQG6Y3AVCNFSM6AAAAABLZRZPCCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENZSG4ZTMNJQGQ . You are receiving this because you were mentioned.Message ID: @.***>