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

feat: Activity dataclass #87

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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 Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ format: .uv
lint: .uv
uv run ruff format --check $(sources)
uv run ruff check $(sources)
uv run mypy $(sources)
uv run --group linting mypy $(sources)
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Ensure consistent usage of dependency groups

The lint target uses the new --group linting flag for mypy but not for ruff commands. This inconsistency could lead to confusion about how dependencies are managed.

Consider updating all linting commands to use the same approach:

.PHONY: lint  ## Lint python source files
lint: .uv
-	uv run ruff format --check $(sources)
-	uv run ruff check $(sources)
-	uv run --group linting mypy $(sources)
+	uv run --group linting ruff format --check $(sources)
+	uv run --group linting ruff check $(sources)
+	uv run --group linting mypy $(sources)

Also, the install target needs to be updated to match the new dependency management approach:

.PHONY: install
install: .uv .pre-commit
-	uv pip install -e ".[dev,linting,testing]"
+	uv pip install -e ".[dev,testing]" --group linting

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


.PHONY: codespell ## Use Codespell to do spellchecking
codespell: .pre-commit
Expand Down
3 changes: 2 additions & 1 deletion garth/data/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
__all__ = ["HRVData", "SleepData"]
__all__ = ["Activity", "HRVData", "SleepData"]

from .activity import Activity
from .hrv import HRVData
from .sleep import SleepData
117 changes: 117 additions & 0 deletions garth/data/activity.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
from datetime import datetime, timezone

from pydantic.dataclasses import dataclass

from .. import http
from ..utils import camel_to_snake_dict, remove_dto_from_dict


@dataclass(frozen=True)
class ActivityType:
type_id: int
type_key: str
parent_type_id: int | None = None


@dataclass(frozen=True)
class Summary:
distance: float
duration: float
moving_duration: float
average_speed: float
max_speed: float
calories: float
average_hr: float
max_hr: float
start_time_gmt: datetime
start_time_local: datetime
start_latitude: float
start_longitude: float
elapsed_duration: float
elevation_gain: float
elevation_loss: float
max_elevation: float
min_elevation: float
average_moving_speed: float
bmr_calories: float
average_run_cadence: float
max_run_cadence: float
average_temperature: float
max_temperature: float
min_temperature: float
average_power: float
max_power: float
min_power: float
normalized_power: float
total_work: float
ground_contact_time: float
stride_length: float
vertical_oscillation: float
training_effect: float
anaerobic_training_effect: float
aerobic_training_effect_message: str
anaerobic_training_effect_message: str
vertical_ratio: float
end_latitude: float
end_longitude: float
max_vertical_speed: float
water_estimated: float
training_effect_label: str
activity_training_load: float
min_activity_lap_duration: float
moderate_intensity_minutes: float
vigorous_intensity_minutes: float
steps: int
begin_potential_stamina: float
end_potential_stamina: float
min_available_stamina: float
avg_grade_adjusted_speed: float
difference_body_battery: float

Comment on lines +16 to +70
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Validate wide-range numeric fields
Summary has many numeric fields (distances, durations, speeds, HR, etc.). Consider adding validation where necessary (e.g., non-negative checks) to safeguard against malformed input.


@dataclass(frozen=True)
class Activity:
activity_id: int
activity_name: str
activity_type: ActivityType
summary: Summary
average_running_cadence_in_steps_per_minute: float | None = None
max_running_cadence_in_steps_per_minute: float | None = None
steps: int | None = None

def _get_localized_datetime(
self, gmt_time: datetime, local_time: datetime
) -> datetime:
local_diff = local_time - gmt_time
local_offset = timezone(local_diff)
gmt_time = datetime.fromtimestamp(
gmt_time.timestamp() / 1000, timezone.utc
)
return gmt_time.astimezone(local_offset)

@property
def activity_start(self) -> datetime:
return self._get_localized_datetime(
self.summary.start_time_gmt, self.summary.start_time_local
)

@classmethod
def get(
cls,
id: int,
*,
client: http.Client | None = None,
):
client = client or http.client
path = f"/activity-service/activity/{id}"
activity_data = client.connectapi(path)
assert activity_data
activity_data = camel_to_snake_dict(activity_data)
activity_data = remove_dto_from_dict(activity_data)
assert isinstance(activity_data, dict)
return cls(**activity_data)

Comment on lines +98 to +113
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Assert usage and error handling
Currently, the method asserts that activity_data is not None and is a dictionary. If either assertion fails, a more descriptive exception or graceful fallback might be helpful.

@classmethod
def list(cls, *args, **kwargs):
data = super().list(*args, **kwargs)
return sorted(data, key=lambda x: x.activity_start)
Comment on lines +114 to +117
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Sorting approach
list returns a sorted list by activity_start. This is straightforward, but consider the performance impact if the dataset grows large. Paginating or lazily fetching might become necessary in the future.

29 changes: 29 additions & 0 deletions garth/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,35 @@ def camel_to_snake_dict(camel_dict: Dict[str, Any]) -> Dict[str, Any]:
return snake_dict


def remove_dto(key: str) -> str:
if key.endswith("_dto"):
return key[: -len("_dto")]
elif key.endswith("DTO"):
return key[: -len("DTO")]
else:
return key


def remove_dto_from_dict(input_dict: Dict[str, Any]) -> Dict[str, Any]:
"""
Removes `DTO` suffix from dictionary keys. Different API endpoints give
back different key names, e.g. "activityTypeDTO" instead of "activityType".
"""
output_dict: Dict[str, Any] = {}
for k, v in input_dict.items():
new_key = remove_dto(k)
if isinstance(v, dict):
output_dict[new_key] = remove_dto_from_dict(v)
elif isinstance(v, list):
output_dict[new_key] = [
remove_dto_from_dict(i) if isinstance(i, dict) else i
for i in v
]
else:
output_dict[new_key] = v
return output_dict

Comment on lines +45 to +63
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Consider edge cases for nested structures
This function recursively removes DTO from keys in nested lists and dictionaries. It seems robust enough, but watch out for cases where internal structures might contain conflicting keys once DTO is removed.


def format_end_date(end: Union[date, str, None]) -> date:
if end is None:
end = date.today()
Expand Down
7 changes: 7 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,13 @@ indent-style = "space"
skip-magic-trailing-comma = false
line-ending = "auto"

[dependency-groups]
linting = [
"mypy>=1.13.0",
"ruff>=0.8.2",
"types-requests>=2.32.0.20241016",
]
Comment on lines +77 to +82
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider consolidating dependency management approach

The new [dependency-groups] section introduces duplicate definitions for linting dependencies that are already defined in [project.optional-dependencies]. This could lead to:

  1. Version conflicts if different constraints are specified
  2. Maintenance overhead from managing dependencies in two places
  3. Confusion about which dependencies are being used

Consider one of these approaches:

  1. Remove the duplicate definitions and use only [dependency-groups]:
-[project.optional-dependencies]
-linting = [
-    "ruff",
-    "mypy",
-    "types-requests",
-]

[dependency-groups]
linting = [
    "mypy>=1.13.0",
    "ruff>=0.8.2",
    "types-requests>=2.32.0.20241016",
]
  1. Or, keep using [project.optional-dependencies] but with the specific versions:
[project.optional-dependencies]
linting = [
-    "ruff",
-    "mypy",
-    "types-requests",
+    "mypy>=1.13.0",
+    "ruff>=0.8.2",
+    "types-requests>=2.32.0.20241016",
]

-[dependency-groups]
-linting = [
-    "mypy>=1.13.0",
-    "ruff>=0.8.2",
-    "types-requests>=2.32.0.20241016",
-]

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


[tool.ruff.lint.isort]
known-first-party = ["garth"]
combine-as-imports = true
Expand Down