open-spaced-repetition / py-fsrs

Python Package for FSRS
https://pypi.org/project/fsrs/
MIT License
145 stars 23 forks source link

datetime.utcnow() deprecated #28

Closed joshdavham closed 6 months ago

joshdavham commented 7 months ago

A Card object's due property is currently initiated using datetime.datetime.utcnow() which is deprecated and set to be removed in a future version of Python.

-> The current recommendation is to instead use datetime.datetime.now(datetime.UTC).


datetime.datetime.utcnow() is said to be a 'naive' datetime as it returns a datetime without any timezone information (e.g., datetime.datetime(2024, 4, 9, 19, 16, 19, 971124))

while datetime.datetime.now(datetime.UTC) is said to be an 'aware' datetime object as it returns a datetime with timezone information included (e.g., datetime.datetime(2024, 4, 9, 19, 16, 19, 971124, tzinfo=datetime.timezone.utc)


I'm happy to open a PR, but this will very likely introduce breaking changes as you currently get an error when you compare 'offset-aware' and 'offset-naive' datetime objects.

For example, I updated my SRS code to use the new datetime recommendation but got an error as my original card object was still offset-naive

`if card_object.due - datetime.now(UTC) >= timedelta(days=1):

...

` => TypeError: can't subtract offset-naive and offset-aware datetimes

If we update datetime, developers can update their old card objects to be timezone aware using card.due = card.due.replace(tzinfo=datetime.timezone.utc) as a sort of hotfix.

What do y'all think?

oschlueter commented 7 months ago

I would argue that it's prudent to move py-fsrs to offset-aware datetimes to prevent it from breaking completely in the future.

As this is a backwards incompatible change, I'd further argue that it warrants a major version bump. Combined with some sort of migration guide, e.g. a two-liner in the changelog, this would be a straightforward change, albeit one that involves some migration effort for py-fsrs users.

Does that sound fair?

L-M-Sherlock commented 7 months ago

I hope this issue could be discussed by some developer who use py-fsrs.

joshdavham commented 7 months ago

I would agree with @oschlueter on this. But we can always wait a bit to get some more input from other developers in the meantime before doing anything.

@L-M-Sherlock If you want, we could wait till the weekend and I can open up a PR then in case we get any more feedback.

joshdavham commented 7 months ago

Update: I'm actually a bit busy the next couple of weeks and all of my mental space is currently devoted to another project.

I should be able to get back to this on May 30th at the latest but, if anyone else wants to grab this in the meantime, go for it!

joshdavham commented 6 months ago

Alrighty I'm back! Will get to work on this this week and hopefully we can resolve the issue

joshdavham commented 6 months ago

PR #31