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

Replace ints and floats with datetime.timedeltas #801

Merged
merged 21 commits into from
Aug 12, 2023

Conversation

MarkZH
Copy link
Collaborator

@MarkZH MarkZH commented Aug 10, 2023

This PR replaces all int and float data that represent time with proper types.

  • Duration: bare int and float replaced by datetime.timedelta
  • Stopwatch: two calls to time.perf_counter() replaced with Timer and calls to time_since_reset()
  • Timer internals: replace time.time() (which can be altered by system clock changes like daylight saving time) with the monotonic time.perf_counter()
  • Variables tagged with _ms, _sec, etc.: explicit conversions to timedelta with functions named msec(), seconds(), etc.

The main benefit of these changes is that there is no need to keep track of what units a given int or float variable is using to store its time. There is no more need for * 1000 or / 1e9 or * 60 operations to get numbers into common units. Everything is handled by the timedelta type. The result of seconds(5) + msec(50) is correctly computed without manually coded conversions.

Other bugs fixed:

  • test_bot/lichess.py line 84: wtime used instead of btime

Note:
The only time variables that were not converted to datetime.timedelta are the time control variables in the functions that create a new challenge (create_challenge() and choose_opponent()). Since these functions select values from lists in the user's config file and then immediately send them to Lichess.challenge(), the round trip from int to timedelta and back to int seemed pointless.

The use of timedeltas is better since the programmer does
not have to keep track of what unit of time is being stored
(ns, ms, sec, min, hrs, etc.). However, the current code is very
verbose thanks to all the datetime.timedelta(seconds=) calls.

Some helper functions are needed to shorten up the code.
datetime.now() can give inaccurate time intervals due to clocks
changing (daylight saving time, leap seconds, NTP synchronization,
etc.). time.time() is not affected by these.
Provide a more compact way of creating datetime.timedeltas.

All variables that hold time duration values are now timedeltas. The
only exception is the Matchmaking method choose_opponent(). Here,
time values are chosen from a configuration file and passed around
unchanged. The conversion from ints to timedeltas and back to ints
seems redundant.
time.monotonic() only increases and is not affected by clock
changes.
The result of datetime.datetime.now() is affected by clock changes
like DST and may not always give accurate timestamps. Use
time.perf_counter() because it is monotonic (never decreases)
and has sub-microsecond resolution.
@AttackingOrDefending
Copy link
Member

Instaed of using timedelta / msec(1) which isn't that easy to understand what it does, we can use:

class Timedelta(datetime.timedelta):
    def total_milliseconds(self) -> float:
        return self / msec(1)

Also, to allow us to use years we can use:

def get_timedelta(years=0, days=0, seconds=0, microseconds=0, milliseconds=0, minutes=0, hours=0, weeks=0):
    return Timedelta(days=years * 365 + days, seconds=seconds, microseconds=microseconds, milliseconds=milliseconds,
                     minutes=minutes, hours=hours, weeks=weeks)

and instead of using import datetime we can use from timer import get_timedelta.

1. to_seconcds() and to_msec() convert a timedelta into a float with
the given time unit. For consistency, to_seconds() replaces
.total_seconds().
2. years() to create t timedelta given a number of years (365 days).
@MarkZH
Copy link
Collaborator Author

MarkZH commented Aug 11, 2023

I opted for a couple more functions instead of a whole new class. It seemed simpler.

Also, the first post of this PR has been updated to explain why the challenge time control variables in the Matchmaking class were not converted to use datetime.timedelta.

@MarkZH MarkZH mentioned this pull request Aug 11, 2023
13 tasks
@MarkZH MarkZH merged commit ad3390f into lichess-bot-devs:master Aug 12, 2023
15 checks passed
@MarkZH MarkZH deleted the timedeltas branch August 12, 2023 08:22
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