feat(typed): add optional Pydantic namespace wrapper (#354)#359
Conversation
Adds ``g.typed`` — a lazy namespace accessor that wraps a curated set of
high-value read endpoints with Pydantic response models, while leaving the
132 existing methods entirely unchanged.
Design follows Option D from the issue discussion:
g = Garmin(email, password)
raw = g.get_stats(date) # dict[str, Any] — unchanged
stats = g.typed.get_stats(date) # DailyStats (Pydantic model)
Scope of the first cut (7 methods, 6 response models):
- get_stats / get_user_summary -> DailyStats
- get_sleep_data -> SleepData (with nested DailySleepDTO)
- get_hrv_data -> HrvData | None
- get_body_battery -> list[BodyBatteryEntry]
- get_training_readiness -> list[TrainingReadiness]
- get_activities_by_date -> list[Activity]
Design notes:
- Lazy import. ``g.typed`` is a ``@functools.cached_property`` that imports
``garminconnect.typed`` on first access, so pydantic stays optional. If
pydantic is missing, the import raises a clear install hint.
- Kept in a new ``[typed]`` extra (pydantic>=2.0.0) rather than promoted to
a core dep, so the default ``pip install garminconnect`` footprint doesn't
change.
- ``extra='allow'`` + all fields ``Optional`` — models tolerate Garmin schema
drift (new firmware / subscription tiers add fields) and partial responses
(privacy-protected accounts, missing device data). Unknown fields land in
``model_extra`` for callers who want them.
- Validation failures raise ``GarminConnectResponseValidationError`` with the
unvalidated response preserved on ``.raw`` so users can fall back without
losing data.
- ``get_training_readiness`` wrapper reflects the real list-shaped response
(the existing ``dict[str, Any]`` annotation on the raw method is wrong but
out of scope here).
Marked **experimental** in module and class docstrings — model shapes may
change between minor releases while the pattern stabilises.
Tests (16 mocked, all green):
- accessor returns cached ``TypedGarmin``
- each wrapper calls through to the raw method and validates the response
- extra fields tolerated (preserved in ``model_extra``)
- missing fields default to ``None``
- validation errors raise ``GarminConnectResponseValidationError`` with
``.raw`` populated
- list endpoints with non-list responses return ``[]``
- ``get_hrv_data`` ``None`` passes through unchanged
Lint clean under ruff + mypy + black + isort.
|
Caution Review failedPull request was closed or merged during review WalkthroughAdds an optional Pydantic-backed typed API layer: new ChangesTyped API Layer
Sequence DiagramsequenceDiagram
actor User
participant GarminClient as Garmin Client
participant TypedWrapper as TypedGarmin Wrapper
participant Pydantic as Pydantic Validator
participant Model as Response Model
User->>GarminClient: access g.typed (first time)
GarminClient->>TypedWrapper: lazily import & construct TypedGarmin(self)
TypedWrapper-->>User: cached TypedGarmin instance
User->>TypedWrapper: .get_stats(date)
TypedWrapper->>GarminClient: .get_stats(date)
GarminClient-->>TypedWrapper: raw dict response
TypedWrapper->>Pydantic: validate(DailyStats, raw)
alt Validation Success
Pydantic->>Model: instantiate DailyStats
Model-->>TypedWrapper: typed model
TypedWrapper-->>User: DailyStats
else Validation Failure
Pydantic-->>TypedWrapper: ValidationError
TypedWrapper->>TypedWrapper: raise GarminConnectResponseValidationError(raw, pydantic_error)
TypedWrapper-->>User: exception with .raw preserved
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 6/8 reviews remaining, refill in 8 minutes and 2 seconds.Comment |
|
Thanks a lot! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pyproject.toml`:
- Around line 80-82: The tests import garminconnect.typed (see
tests/test_typed.py) which requires pydantic but pydantic is only listed under
the "typed" extra in pyproject.toml; update pyproject.toml to add
"pydantic>=2.0.0" to the testing/dev/test dependencies group so running pytest
with only the testing extras installed will not fail during collection—ensure
the exact pydantic version constraint used in the "typed" extra is duplicated in
the testing dependencies.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 628650ac-72c4-4d24-8752-89bfa3187c63
📒 Files selected for processing (4)
garminconnect/__init__.pygarminconnect/typed.pypyproject.tomltests/test_typed.py
| typed = [ | ||
| "pydantic>=2.0.0", | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
import tomllib, pathlib
cfg = tomllib.loads(pathlib.Path("pyproject.toml").read_text())
proj = cfg["project"]["optional-dependencies"]
dev = cfg.get("tool", {}).get("pdm", {}).get("dev-dependencies", {})
print("project.optional-dependencies.testing =", proj.get("testing", []))
print("project.optional-dependencies.typed =", proj.get("typed", []))
print("tool.pdm.dev-dependencies.testing =", dev.get("testing", []))
print("testing has pydantic:", any("pydantic" in x for x in proj.get("testing", [])))
print("dev-testing has pydantic:", any("pydantic" in x for x in dev.get("testing", [])))
PY
rg -n "from garminconnect\.typed import|import garminconnect\.typed" tests
rg -n -C2 --iglob "*.yml" "pdm install|pip install" .github/workflows || trueRepository: cyberjunky/python-garminconnect
Length of output: 1186
🏁 Script executed:
# Check what garminconnect/typed.py imports and requires
head -30 garminconnect/typed.py
# Check test_typed.py for any conditional imports or skip decorators
head -50 tests/test_typed.py
# Check if there are any pytest markers or conditions on test_typed.py
grep -n "pytest.mark\|skipif\|skip\|importorskip" tests/test_typed.py || echo "No skip markers found"Repository: cyberjunky/python-garminconnect
Length of output: 2803
🏁 Script executed:
# Check if garminconnect/typed.py imports pydantic
grep -n "^from pydantic\|^import pydantic" garminconnect/typed.py
# Check if there are any class definitions using Pydantic BaseModel
grep -n "class.*BaseModel\|class.*ValidationError" garminconnect/typed.py | head -10Repository: cyberjunky/python-garminconnect
Length of output: 406
🏁 Script executed:
# Check the actual imports in garminconnect/typed.py
head -100 garminconnect/typed.py | grep -n "from pydantic\|^import pydantic"Repository: cyberjunky/python-garminconnect
Length of output: 204
🏁 Script executed:
# Check context around pydantic imports to confirm they're unconditional
sed -n '30,45p' garminconnect/typed.pyRepository: cyberjunky/python-garminconnect
Length of output: 577
Add pydantic to testing dependencies for improved developer experience.
tests/test_typed.py imports garminconnect.typed at module level (line 30), which requires pydantic. Currently, pydantic is only in the typed extra—not in testing deps. This means test collection fails if an environment installs only testing dependencies without the typed extra. While the error message is clear, adding pydantic to testing deps allows test suites to run with just the testing group installed, improving local developer experience and simplifying dependency management.
Proposed fix
[project.optional-dependencies]
testing = [
"coverage",
"pytest",
"pytest-vcr>=1.0.2",
"vcrpy>=7.0.0",
+ "pydantic>=2.0.0",
]
@@
[tool.pdm.dev-dependencies]
testing = [
"coverage",
"pytest",
"pytest-vcr>=1.0.2",
"vcrpy>=7.0.0",
+ "pydantic>=2.0.0",
]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pyproject.toml` around lines 80 - 82, The tests import garminconnect.typed
(see tests/test_typed.py) which requires pydantic but pydantic is only listed
under the "typed" extra in pyproject.toml; update pyproject.toml to add
"pydantic>=2.0.0" to the testing/dev/test dependencies group so running pytest
with only the testing extras installed will not fail during collection—ensure
the exact pydantic version constraint used in the "typed" extra is duplicated in
the testing dependencies.
There was a problem hiding this comment.
Pull request overview
Adds an optional g.typed namespace that exposes a small set of high-value Garmin read endpoints as Pydantic-validated response models, while keeping the existing dict/list-returning API unchanged.
Changes:
- Introduces
garminconnect.typedwith Pydantic models + aTypedGarminwrapper and a dedicated validation error that preserves the raw payload. - Adds
Garmin.typedas a lazy, cached accessor to the typed wrapper (keeping Pydantic as an optional dependency). - Adds a mocked test suite validating delegation, schema tolerance, and validation-failure behavior for the typed endpoints.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
garminconnect/typed.py |
New Pydantic response models + TypedGarmin wrapper and validation error type. |
garminconnect/__init__.py |
Adds Garmin.typed cached_property with lazy import to keep Pydantic optional. |
pyproject.toml |
Adds a new typed extra depending on pydantic>=2.0.0. |
tests/test_typed.py |
New unit tests for the typed wrapper behavior and validation policy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| typed = [ | ||
| "pydantic>=2.0.0", | ||
| ] |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Closes #354 (once merged).
Implements Option D from the issue discussion: a lazy
g.typednamespace that wraps a small set of high-value read endpoints with Pydantic response models, while leaving the 132 existing methods entirely untouched.Opening as draft so you can veto any of the design choices below before I polish / land.
Decisions I made (please flag any you'd change)
GarminConnectResponseValidationErrorwith the unvalidated response preserved as.rawtry/exceptand still access the data. The alternative (silent fallback to dict) was the one open UX question you hadn't weighed in on — happy to flip to silent-fallback if you'd prefer.extra='allow'+ all fieldsOptional[typed]extra, not core depg.typedlazy-imports the module and raises a clear install hint if pydantic is missingpopulate_by_name=Truelist[TrainingReadiness]Garmin.get_training_readinessis annotateddict[str, Any]Scope
get_stats,get_user_summaryDailyStatsget_sleep_dataSleepData(nestedDailySleepDTO)get_hrv_dataHrvData | Noneget_body_batterylist[BodyBatteryEntry]get_training_readinesslist[TrainingReadiness]get_activities_by_datelist[Activity]Tests
16 new mocked tests in
tests/test_typed.py. All 107 mock-based tests still pass.Covered scenarios:
TypedGarminmodel_extra(no validation failure)NoneGarminConnectResponseValidationErrorwith.raw[]get_hrv_dataNonepasses through unchangedOut of scope (follow-ups)
dict[str, Any]annotation onget_training_readiness— separate PR, doesn't need to block this.pragma: no cover; happy to add an integration test if you want.Verification
pdm run ruff check garminconnect/typed.py tests/test_typed.py— cleanpdm run mypy garminconnect/typed.py— cleanblack -l 88 --target-version py313— cleanisort --profile black— cleanpytest tests/test_typed.py— 16 passedtest_garmin_unit,test_retry_decorator,test_workout_constants,test_typed) — 107 passed, 0 regressionsLet me know if the direction is off and I'll rework.
Summary by CodeRabbit
New Features
Tests
Chore