Add additional metrics - Hill + Endurance score, Morning Training Readiness #135
Conversation
WalkthroughThe changes introduce two new data classes, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client
participant GarminAPI
User->>Client: Call GarminScoresData.get(date)
Client->>GarminAPI: Fetch hill score for date
GarminAPI-->>Client: Hill score data
Client->>GarminAPI: Fetch endurance score for date
GarminAPI-->>Client: Endurance score data
Client->>User: Return GarminScoresData instance
User->>Client: Call MorningTrainingReadinessData.get(date)
Client->>GarminAPI: Fetch readiness data for date
GarminAPI-->>Client: Readiness data (multiple entries)
Client->>Client: Filter for 'AFTER_WAKEUP_RESET'
Client->>User: Return MorningTrainingReadinessData instance
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/garth/data/garmin_scores.py (2)
38-46: Consider API call efficiency and error handlingThe method makes two separate API calls which could be inefficient, but this may be necessary due to the API design. The error handling returns
Noneif either endpoint fails, which is reasonable but could be more granular.Consider if both endpoints are always required or if partial data could be useful in some cases.
60-65: Complex field renaming logic - consider adding commentsThe dictionary comprehension for renaming endurance classification fields is complex but appears correct. Consider adding a comment to explain the logic for better maintainability.
+ # Rename classification fields to include 'endurance_classification' prefix data_endurance_score = { ("endurance_classification" + key[len("classification") :]) if key.startswith("classification") else key: value for key, value in data_endurance_score.items() }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
tests/data/cassettes/test_garmin_scores_data_get.yamlis excluded by!tests/**/cassettes/**tests/data/cassettes/test_garmin_scores_data_list.yamlis excluded by!tests/**/cassettes/**tests/data/cassettes/test_morning_training_readiness_data_get.yamlis excluded by!tests/**/cassettes/**tests/data/cassettes/test_morning_training_readiness_data_list.yamlis excluded by!tests/**/cassettes/**
📒 Files selected for processing (7)
README.md(1 hunks)src/garth/__init__.py(2 hunks)src/garth/data/__init__.py(2 hunks)src/garth/data/garmin_scores.py(1 hunks)src/garth/data/morning_training_readiness.py(1 hunks)tests/data/test_garmin_scores.py(1 hunks)tests/data/test_morning_training_readiness.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`tests/**`: - test functions shouldn't have a return type hint - it's ok to use `assert` instead of `pytest.assume()`
tests/**: - test functions shouldn't have a return type hint
- it's ok to use
assertinstead ofpytest.assume()
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
tests/data/test_garmin_scores.pytests/data/test_morning_training_readiness.py
🧠 Learnings (2)
tests/data/test_garmin_scores.py (1)
Learnt from: matin
PR: matin/garth#76
File: tests/test_http.py:109-109
Timestamp: 2024-12-07T14:07:11.531Z
Learning: In Python test files (e.g., `tests/test_http.py`), top-level test functions should be separated by two blank lines, as per PEP 8 style guidelines.
tests/data/test_morning_training_readiness.py (1)
Learnt from: matin
PR: matin/garth#76
File: tests/test_http.py:109-109
Timestamp: 2024-12-07T14:07:11.531Z
Learning: In Python test files (e.g., `tests/test_http.py`), top-level test functions should be separated by two blank lines, as per PEP 8 style guidelines.
🧬 Code Graph Analysis (2)
src/garth/__init__.py (3)
src/garth/data/garmin_scores.py (1)
GarminScoresData(14-74)src/garth/data/hrv.py (1)
HRVData(39-68)src/garth/data/morning_training_readiness.py (1)
MorningTrainingReadinessData(15-69)
src/garth/data/morning_training_readiness.py (3)
src/garth/utils.py (1)
camel_to_snake_dict(17-33)src/garth/data/_base.py (1)
Data(15-47)src/garth/http.py (2)
Client(19-244)connectapi(186-192)
🔇 Additional comments (13)
src/garth/__init__.py (2)
4-6: LGTM - Imports follow existing patternsThe new imports are correctly placed and follow the established alphabetical ordering within the import block.
36-38: LGTM - Exports properly added to allThe new classes are correctly added to the
__all__export list in alphabetical order, maintaining consistency with existing patterns.README.md (1)
908-1068: LGTM - Comprehensive documentation for new featuresThe new documentation sections are well-structured and follow the existing style patterns. The examples demonstrate both single-date and multi-date usage scenarios with detailed sample outputs, which will be helpful for users implementing these features.
src/garth/data/__init__.py (1)
6-8: LGTM - Consistent with existing module patternsThe new exports and imports are correctly added following the established alphabetical ordering and import structure patterns.
Also applies to: 21-23
src/garth/data/garmin_scores.py (2)
14-29: LGTM - Well-structured dataclass definitionThe dataclass fields are comprehensive and well-named, covering all the metrics from both hill and endurance score endpoints. The field names are descriptive and follow Python naming conventions.
71-74: LGTM - Consistent list method implementationThe list method correctly delegates to the parent class and sorts by calendar_date, which is consistent with other data classes in the codebase.
tests/data/test_garmin_scores.py (2)
9-24: LGTM - Comprehensive test coverage for get() methodThe test correctly verifies both positive and negative cases:
- Tests successful data retrieval with assertions on key fields
- Tests the case where no data is available (returns None)
- Follows the established testing patterns with
pytest.mark.vcrandauthed_clientfixture
26-34: LGTM - Appropriate test for list() methodThe test verifies the list functionality correctly:
- Tests with appropriate parameters (days=2, max_workers=1)
- Verifies the returned data length matches expected days
- Checks that the last entry matches the expected end date
- Uses single worker to ensure deterministic test execution
tests/data/test_morning_training_readiness.py (1)
26-34: Test implementation looks good.The test properly validates the list method functionality, checking both the returned length and the last entry's date.
src/garth/data/morning_training_readiness.py (4)
1-12: Imports and setup look good.The imports are appropriately scoped and the module structure follows the established pattern in the codebase.
14-35: Well-structured dataclass with comprehensive field definitions.The dataclass appropriately captures all the morning training readiness metrics with correct type annotations. The field names follow Python naming conventions.
37-64: Robust implementation of the get method.The method properly:
- Handles the case when no data is returned from the API
- Filters for the specific "AFTER_WAKEUP_RESET" context to isolate morning readiness data
- Uses the established
camel_to_snake_dictutility for field conversion- Follows the pattern established in other data classes
66-69: Good override of the list method.The method appropriately sorts the results by calendar_date, which provides a consistent and logical ordering for time-series data.
vactomas
force-pushed
the
feature/add-additional-garmin-scores
branch
from
840a25b to
5d23551
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #135 +/- ##
===========================================
- Coverage 100.00% 99.95% -0.05%
===========================================
Files 45 49 +4
Lines 1898 2028 +130
===========================================
+ Hits 1898 2027 +129
- Misses 0 1 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@vactomas both of your PRs have the same issue. let's put you could also mock the response, but don't worry about it. let's get these PRs merged |
Hi, matin,
I've added a couple of additional metrics, which can be obtained from the Garmin API. I believe they could be of use to people, who process the data themselves.
Let me know, what you think!
Summary by CodeRabbit
New Features
Documentation
Tests