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

Date/DateTime + relativedelta() #11

Open
SonOfLilit opened this issue Aug 1, 2024 · 6 comments
Open

Date/DateTime + relativedelta() #11

SonOfLilit opened this issue Aug 1, 2024 · 6 comments

Comments

@SonOfLilit
Copy link

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
Copy link
Author

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

SonOfLilit@7690b5b

@glyph
Copy link
Owner

glyph commented Aug 1, 2024

@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
Copy link
Owner

glyph commented Aug 1, 2024

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
Copy link
Author

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
Copy link
Owner

glyph commented Aug 7, 2024

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
Copy link
Author

SonOfLilit commented Aug 8, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants