-
-
Notifications
You must be signed in to change notification settings - Fork 209
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
Make stats time code consistent #1625
Conversation
Some stats used to be for "last" period whereas other stats considered the "ongoing" period till the current date i.e. last week vs 7 days ago to today. All entity stats now use the last week/month/year periods. Also refactored the code a bit for de-duplicating stuff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there with the revamp -- well done!
|
||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
def get_time_range(stats_range: str) -> Tuple[datetime, datetime, relativedelta, str]: | ||
latest_listen_ts = get_latest_listen_ts() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A shame that this function is close, but not really close to get_dates_for_stats_range....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed :(
Clarify what periods stats are calcualted for and make them consistent across all type/range of stats. The stats are to be calculated for the "last" period as per discussion in IRC. Comments in code describe what this means. We should probably explain this on the website as well so that users know when to expect a stats update and why their current listens are no longer reflecting in the stats the next day.