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

remove pendulum #492

Merged
merged 2 commits into from
Apr 15, 2024
Merged

remove pendulum #492

merged 2 commits into from
Apr 15, 2024

Conversation

deronnax
Copy link
Contributor

@deronnax deronnax commented Apr 11, 2024

As it is a pain: pendulum 2 is incompatible with python 3.12 and pendulum 3 has strange build errors we do not understand. The package has governance problems were the sole maintainer won't address problems, and more problematically, won't even answer to questions, see sdispater/pendulum#771.

Python-dateutil is a more reliable package to rely on: it's been around for longer, is not a one-person project and the founder is now a python core-maintainer in charge of the datetime module.

@deronnax deronnax force-pushed the drop-pendulum branch 6 times, most recently from 966fc3d to 07e43db Compare April 11, 2024 12:39
@deronnax
Copy link
Contributor Author

deronnax commented Apr 11, 2024

it looks pendulum.parse for a date assumes UTC, so I had to force UTC timezone on datetime.fromisoformat. I doubt this is a good thing and I think the python datetime module knows better than pendulum, but @laixintao you have the final say.

@deronnax deronnax closed this Apr 11, 2024
@deronnax deronnax reopened this Apr 11, 2024
@deronnax deronnax mentioned this pull request Apr 11, 2024
Copy link
Owner

@laixintao laixintao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks PR is great!

I agree to remove pendulum, but can you double check whether we can remove python-dateutil as well?

for unit, minimum in self.when_lower_than.items():
if current <= minimum:
if self.future_time:
dt = now.add(**{f"{unit}s": current})
dt = now + relativedelta(**{f"{unit}s": current})
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible we can use builtin timedelta?

from datetime import tiemdelta

dt = now + timedelta(**{f"{unit}s": current})

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That what I went with first, but timedelta doesn't handle month or year units, so I went with python-dateutils.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, got it!

@deronnax
Copy link
Contributor Author

We can't simply remove python-dateutil (but it's not an addition: it was already used underneath by pendulum). Iredis would not be able to do good future/past completion in time without it, thanks to its superior relativedelta.

@laixintao laixintao merged commit 499769a into laixintao:master Apr 15, 2024
36 checks passed
@laixintao
Copy link
Owner

Thank you so so much!

@laixintao
Copy link
Owner

Released v1.15.0

@deronnax
Copy link
Contributor Author

You're welcome. Thank you :)

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

Successfully merging this pull request may close these issues.

2 participants