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.

Sign up for GitHub

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

chore: update version to 1.4.15 and add timezone support in date hand… #40

Merged
merged 3 commits into from
Jan 20, 2025
Merged

chore: update version to 1.4.15 and add timezone support in date hand… #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
81130e1
chore: update version to 1.4.15 and add timezone support in date hand…
amindadgar Jan 20, 2025
2726bf0
feat: add timezone support to analyzer_period in TCAnalyzer
amindadgar Jan 20, 2025
4ccd1ff
feat: add timezone handling for member activity dates
amindadgar Jan 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

setup(
name="tc-analyzer-lib",
version="1.4.14",
version="1.4.15",
author="Mohammad Amin Dadgar, TogetherCrew",
maintainer="Mohammad Amin Dadgar",
maintainer_email="[email protected]",
Expand Down
9 changes: 3 additions & 6 deletions tc_analyzer_lib/algorithms/member_activity_history.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# checking the past history of member activities

from datetime import datetime, timedelta
from datetime import datetime, timedelta, timezone

from tc_analyzer_lib.algorithms.utils.member_activity_history_utils import (
MemberActivityPastUtils,
Expand Down Expand Up @@ -106,11 +106,8 @@ def check_past_history(
# db_analysis_start_date = None
db_analysis_end_date = None

# # the input date_range in format of datetime
# # converting the dates into datetime format
# date_format = "%y/%m/%d"
# date_range_start = datetime.datetime.strptime(date_range[0], date_format)
# date_range_end = datetime.datetime.strptime(date_range[1], date_format)
if db_analysis_end_date:
db_analysis_end_date = db_analysis_end_date.replace(tzinfo=timezone.utc)
Comment on lines +109 to +110
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure consistent timezone handling for all datetime objects.

While db_analysis_end_date is now timezone-aware, the comparisons with date_range_start and date_range_end could raise TypeError if they are naive datetime objects.

Apply this fix to ensure all datetime comparisons are timezone-aware:

    if db_analysis_end_date:
        db_analysis_end_date = db_analysis_end_date.replace(tzinfo=timezone.utc)
+
+   # Ensure date range is timezone-aware
+   date_range_start = date_range_start.replace(tzinfo=timezone.utc) if date_range_start.tzinfo is None else date_range_start
+   date_range_end = date_range_end.replace(tzinfo=timezone.utc) if date_range_end.tzinfo is None else date_range_end

Committable suggestion skipped: line range outside the PR's diff.


new_date_range: list[datetime]
# if for the requested date_range, its results were available in db
Expand Down
11 changes: 8 additions & 3 deletions tc_analyzer_lib/metrics/analyzer_memberactivities.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import logging
from datetime import datetime, timedelta
from datetime import datetime, timedelta, timezone

from tc_analyzer_lib.algorithms.compute_member_activity import compute_member_activity
from tc_analyzer_lib.metrics.memberactivity_utils import MemberActivityUtils
Expand Down Expand Up @@ -73,7 +73,9 @@ def analysis_member_activity(
return (None, None)

# get date range to be analyzed
today = datetime.now().replace(hour=0, minute=0, second=0, microsecond=0)
today = datetime.now().replace(
hour=0, minute=0, second=0, microsecond=0, tzinfo=timezone.utc
)

logging.info(f"{guild_msg} memberactivities Analysis started!")

Expand All @@ -87,7 +89,7 @@ def analysis_member_activity(
load_past_data = load_past_data and not from_start

first_date = self.analyzer_period.replace(
hour=0, minute=0, second=0, microsecond=0
hour=0, minute=0, second=0, microsecond=0, tzinfo=timezone.utc
)
if first_date is None:
logging.error(
Expand Down Expand Up @@ -136,6 +138,9 @@ def analysis_member_activity(
if not from_start:
# first date of storing the data
first_storing_date = member_activity_c.get_last_date()
if first_storing_date:
first_storing_date = first_storing_date.replace(tzinfo=timezone.utc)

activities = self.utils.refine_memberactivities_data(
activities, first_storing_date
)
Expand Down
3 changes: 2 additions & 1 deletion tc_analyzer_lib/models/MemberActivityModel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#!/usr/bin/env python3
import logging
from datetime import datetime

import pymongo
from tc_analyzer_lib.models.BaseModel import BaseModel
Expand All @@ -12,7 +13,7 @@ def __init__(self, database=None):
raise Exception("Database should not be None")
super().__init__(collection_name="memberactivities", database=database)

def get_last_date(self):
def get_last_date(self) -> datetime | None:
"""
Gets the date of the last document
"""
Expand Down
6 changes: 3 additions & 3 deletions tc_analyzer_lib/tc_analyzer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import logging
from datetime import datetime
from datetime import datetime, timezone

from tc_analyzer_lib.metrics.analyzer_memberactivities import MemberActivities
from tc_analyzer_lib.metrics.heatmaps import Heatmaps
Expand Down Expand Up @@ -105,7 +105,7 @@ async def run_once(self):
action_config=self.action,
window_config=self.window,
analyzer_config=self.analyzer_config,
analyzer_period=self.period,
analyzer_period=self.period.replace(tzinfo=timezone.utc),
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Timezone handling needs to be consistent

The verification confirms that period can be timezone-naive when passed to the constructor. For example, in setup_platform.py, the period is created without timezone information. Adding timezone.utc in run_once is correct, but we should also ensure timezone awareness in the constructor to maintain consistency.

  • Consider adding timezone validation in TCAnalyzer.__init__ to ensure period is always timezone-aware
  • Update test utilities like setup_platform.py to create timezone-aware datetime objects
🔗 Analysis chain

Verify timezone handling in constructor

While adding UTC timezone here is correct, we should ensure that self.period is consistently handled when it's initially set in the constructor.

Let's verify the timezone handling in the constructor and any potential callers:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for TCAnalyzer instantiations to verify period parameter handling
rg -l "TCAnalyzer\(" | xargs rg -B 5 -A 1 "TCAnalyzer\("

# Search for period parameter assignments in constructor
ast-grep --pattern 'class TCAnalyzer {
  $$$
  __init__($$$) {
    $$$
    self.period = period
    $$$
  }
}'

Length of output: 1543

Script:

#!/bin/bash
# Get the complete constructor implementation
ast-grep --pattern 'class TCAnalyzer {
  $$$
  def __init__($$$) {
    $$$
  }
}'

# Find period parameter usage in setup_platform.py
rg -B 5 -A 5 "TCAnalyzer\(" tests/integration/utils/setup_platform.py

# Look for period type hints or related imports
rg "from datetime" tc_analyzer_lib/tc_analyzer.py

Length of output: 506

Script:

#!/bin/bash
# Try different pattern for constructor
ast-grep --pattern 'class TCAnalyzer($_) {
  def __init__(self, $$$) {
    $$$
  }
}'

# Find period variable definition in setup_platform
rg -B 10 "period=" tests/integration/utils/setup_platform.py

# Look for period-related type hints
rg "period.*:" tc_analyzer_lib/tc_analyzer.py

Length of output: 579

Script:

#!/bin/bash
# Find period variable definition with more context
rg -B 15 -A 5 "days_ago_period" tests/integration/utils/setup_platform.py

# Try one more pattern for constructor
ast-grep --pattern 'class TCAnalyzer {
  def __init__(self, platform_id: str, period: datetime, $$$) {
    $$$
  }
}'

Length of output: 2328

)
(
member_activities_data,
Expand Down Expand Up @@ -190,7 +190,7 @@ async def recompute(self):
action_config=self.action,
window_config=self.window,
analyzer_config=self.analyzer_config,
analyzer_period=self.period,
analyzer_period=self.period.replace(tzinfo=timezone.utc),
)
(
member_activities_data,
Expand Down
Loading