Repository: stravalib/stravalib
Pull request #526 (2024-06-15 08:04:35)

Gear and Athlete type hierarchies (+ naive_datetime typing fixes)
This corrects the type hierarchies for `Gear` and `Athlete` types, + several other typing issues I encountered in the `pydantic-v2` branch. The remaining (3) typing issues will be resolved by #510 .

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/stravalib/stravalib/pull/526?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stravalib) Report
Attention: Patch coverage is `93.10345%` with `2 lines` in your changes missing coverage. Please review.
> Please [upload](https://docs.codecov.com/docs/codecov-uploader) report for BASE (`pydantic-v2@3174656`). [Learn more](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stravalib#section-missing-base-commit) about missing BASE report.

| [Files](https://app.codecov.io/gh/stravalib/stravalib/pull/526?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stravalib) | Patch % | Lines |
|---|---|---|
| [src/stravalib/model.py](https://app.codecov.io/gh/stravalib/stravalib/pull/526?src=pr&el=tree&filepath=src%2Fstravalib%2Fmodel.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stravalib#diff-c3JjL3N0cmF2YWxpYi9tb2RlbC5weQ==) | 91.66% | [1 Missing and 1 partial :warning: ](https://app.codecov.io/gh/stravalib/stravalib/pull/526?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stravalib) |

<details><summary>Additional details and impacted files</summary>


```diff
@@              Coverage Diff               @@
##             pydantic-v2     #526   +/-   ##
==============================================
  Coverage               ?   89.38%           
==============================================
  Files                  ?       10           
  Lines                  ?     1818           
  Branches               ?      171           
==============================================
  Hits                   ?     1625           
  Misses                 ?      152           
  Partials               ?       41           
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/stravalib/stravalib/pull/526?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stravalib).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=stravalib).


Patch:
diff --git a/src/stravalib/client.py b/src/stravalib/client.py
index 0a9993d..7e45407 100644
--- a/src/stravalib/client.py
+++ b/src/stravalib/client.py
@@ -308,7 +308,7 @@ class Client:
             limit=limit,
         )
 
-    def get_athlete(self) -> model.Athlete:
+    def get_athlete(self) -> model.DetailedAthlete:
         """Gets the specified athlete; if athlete_id is None then retrieves a
         detail-level representation of currently authenticated athlete;
         otherwise summary-level representation returned of athlete.
@@ -322,13 +322,15 @@ class Client:
 
         Returns
         -------
-        class:`stravalib.model.Athlete`
+        class:`stravalib.model.DetailedAthlete`
             The athlete model object.
 
         """
         raw = self.protocol.get("/athlete")
 
-        return model.Athlete.model_validate({**raw, **{"bound_client": self}})
+        return model.DetailedAthlete.model_validate(
+            {**raw, **{"bound_client": self}}
+        )
 
     def update_athlete(
         self,
@@ -337,7 +339,7 @@ class Client:
         country: str | None = None,
         sex: str | None = None,
         weight: float | None = None,
-    ) -> model.Athlete:
+    ) -> model.DetailedAthlete:
         """Updates the properties of the authorized athlete.
 
         https://developers.strava.com/docs/reference/#api-Athletes-updateLoggedInAthlete
@@ -383,7 +385,7 @@ class Client:
             params["weight"] = float(weight)
 
         raw_athlete = self.protocol.put("/athlete", **params)
-        return model.Athlete.model_validate(
+        return model.DetailedAthlete.model_validate(
             {**raw_athlete, **{"bound_client": self}}
         )
 
@@ -1026,9 +1028,7 @@ class Client:
 
         return ActivityUploader(self, response=initial_response)
 
-    def get_activity_zones(
-        self, activity_id: int
-    ) -> list[model.BaseActivityZone]:
+    def get_activity_zones(self, activity_id: int) -> list[model.ActivityZone]:
         """Gets activity zones for activity.
 
         Activity zones relate to a users effort (heartrate and power).
@@ -1043,7 +1043,7 @@ class Client:
         Returns
         -------
         :class:`list`
-            A list of :class:`stravalib.model.BaseActivityZone` objects.
+            A list of :class:`stravalib.model.ActivityZone` objects.
 
         Notes
         -----
@@ -1055,9 +1055,7 @@ class Client:
         zones = self.protocol.get("/activities/{id}/zones", id=activity_id)
 
         return [
-            model.BaseActivityZone.model_validate(
-                {**z, **{"bound_client": self}}
-            )
+            model.ActivityZone.model_validate({**z, **{"bound_client": self}})
             for z in zones
         ]
 
@@ -1195,7 +1193,7 @@ class Client:
         Returns
         -------
         class:`BatchedResultsIterator`
-            An iterator of :class:`stravalib.model.ActivityLaps` objects.
+            An iterator of :class:`stravalib.model.Lap` objects.
 
         """
         result_fetcher = functools.partial(
@@ -1208,7 +1206,7 @@ class Client:
             result_fetcher=result_fetcher,
         )
 
-    def get_gear(self, gear_id: str) -> model.Gear:
+    def get_gear(self, gear_id: str) -> strava_model.DetailedGear:
         """Get details for an item of gear.
 
         https://developers.strava.com/docs/reference/#api-Gears
@@ -1220,11 +1218,10 @@ class Client:
 
         Returns
         -------
-        class:`stravalib.model.Gear`
-            The Bike or Shoe subclass object.
+        class:`stravalib.strava_model.DetailedGear`
 
         """
-        return model.Gear.model_validate(
+        return strava_model.DetailedGear.model_validate(
             self.protocol.get("/gear/{id}", id=gear_id)
         )
 
diff --git a/src/stravalib/model.py b/src/stravalib/model.py
index 3dbb15b..8f9df76 100644
--- a/src/stravalib/model.py
+++ b/src/stravalib/model.py
@@ -28,6 +28,7 @@ from typing import (
 )
 
 from dateutil import parser
+from dateutil.parser import ParserError
 from pydantic import (
     AliasChoices,
     BaseModel,
@@ -40,19 +41,15 @@ from stravalib import exc, strava_model
 from stravalib.strava_model import (
     ActivityTotal,
     ActivityType,
-    ActivityZone,
     BaseStream,
     Comment,
-    DetailedGear,
     ExplorerSegment,
     LatLng,
     PhotosSummary,
     PolylineMap,
     Primary,
     SportType,
-    SummaryAthlete,
     SummaryGear,
-    TimedZoneRange,
 )
 
 if TYPE_CHECKING:
@@ -64,6 +61,16 @@ T = TypeVar("T")
 U = TypeVar("U", bound="BoundClientEntity")
 
 
+# Create alias for this type so docs are more readable
+AllDateTypes = Union[
+    datetime,
+    str,
+    bytes,
+    int,
+    float,
+]
+
+
 # Could naive_datetime also be run in a validator?
 def naive_datetime(value: Optional[AllDateTypes]) -> Optional[datetime]:
     """Utility helper that parses a datetime value provided in
@@ -87,22 +94,32 @@ def naive_datetime(value: Optional[AllDateTypes]) -> Optional[datetime]:
 
     YYYY-MM-DD[T]HH:MM[:SS[.ffffff]][Z or [±]HH[:]MM]
     """
-    if not value:
-        return None
 
-    # Allow for timestamps to be passed as strings, e.g '1714305600'.
-    try:
-        value_is_int = isinstance(value, int) or float(value).is_integer()
-    except (ValueError, TypeError):
-        value_is_int = False
+    if value is None:
+        return None
 
-    dt = (
-        parser.parse(str(value))
-        if not value_is_int
-        else datetime.fromtimestamp(int(value))
-    )
-    # Remove timezone information, if any
-    return dt.replace(tzinfo=None)
+    if isinstance(value, (int, float)):
+        # If epoch is given, we have to assume it's UTC. When using the
+        # regular fromtimestamp(), epoch will be interpreted as in the
+        # _local_ timezone.
+        dt = datetime.utcfromtimestamp(value)
+        return dt.replace(tzinfo=None)
+    elif isinstance(value, str):
+        try:
+            dt = parser.parse(value)
+            return dt.replace(tzinfo=None)
+        except ParserError:
+            # Maybe timestamp was passed as string, e.g '1714305600'?
+            try:
+                dt = datetime.utcfromtimestamp(float(value))
+                return dt.replace(tzinfo=None)
+            except ValueError:
+                LOGGER.error(f"Invalid datetime value: {value}")
+                raise
+    elif isinstance(value, datetime):
+        return value.replace(tzinfo=None)
+    else:
+        raise ValueError(f"Unsupported value type: {type(value)}")
 
 
 def lazy_property(fn: Callable[[U], T]) -> Optional[T]:
@@ -200,10 +217,6 @@ def check_valid_location(
         return location if location else None
 
 
-# Create alias for this type so docs are more readable
-AllDateTypes = Union[datetime, str, bytes, int, float]
-
-
 class BoundClientEntity(BaseModel):
     """A class that bounds the Client object to the model."""
 
@@ -348,7 +361,7 @@ class SummaryClub(MetaClub, strava_model.SummaryClub):
     club_type: Optional[str] = None
 
     @lazy_property
-    def members(self) -> BatchedResultsIterator[Athlete]:
+    def members(self) -> BatchedResultsIterator[strava_model.ClubAthlete]:
         """
         Lazy property to retrieve club members stored as Athlete objects.
 
@@ -378,32 +391,6 @@ class DetailedClub(SummaryClub, strava_model.DetailedClub):
     pass
 
 
-class Gear(DetailedGear):
-    """
-    Represents a piece of gear (equipment) used in physical activities.
-    """
-
-    pass
-
-
-class Bike(Gear):
-    """
-    Represents a bike as a "type" / using the structure of
-    `stravalib.model.Gear`.
-    """
-
-    pass
-
-
-class Shoe(Gear):
-    """
-    Represents a Shoes as a "type" / using the structure of
-    `stravalib.model.Gear`.
-    """
-
-    pass
-
-
 class ActivityTotals(ActivityTotal):
     """An objecting containing a set of total values for an activity including
     elapsed time, moving time, distance and elevation gain."""
@@ -434,8 +421,10 @@ class MetaAthlete(strava_model.MetaAthlete, BoundClientEntity):
     resource_state: Optional[int] = None
 
 
-# TODO: rename to detailed athlete
-class Athlete(strava_model.DetailedAthlete):
+class SummaryAthlete(MetaAthlete, strava_model.SummaryAthlete): ...
+
+
+class DetailedAthlete(SummaryAthlete, strava_model.DetailedAthlete):
     """Represents high level athlete information including
     their name, email, clubs they belong to, bikes, shoes, etc.
 
@@ -448,8 +437,6 @@ class Athlete(strava_model.DetailedAthlete):
 
     # Field overrides from superclass for type extensions:
     clubs: Optional[list[SummaryClub]] = None
-    bikes: Optional[list[SummaryGear]] = None
-    shoes: Optional[list[SummaryGear]] = None
 
     # Undocumented attributes:
     athlete_type: Optional[Literal["cyclist", "runner"]] = None
@@ -652,9 +639,10 @@ class Lap(
     strava_model.Lap,
     BoundClientEntity,
 ):
+
     # Field overrides from superclass for type extensions:
     activity: Optional[MetaActivity] = None
-    athlete: Optional[Athlete] = None
+    athlete: Optional[MetaAthlete] = None
 
     # Undocumented attributes:
     average_watts: Optional[float] = None
@@ -823,7 +811,6 @@ class SummarySegmentEffort(strava_model.SummarySegmentEffort):
 class BaseEffort(
     SummarySegmentEffort,
     strava_model.DetailedSegmentEffort,
-    BoundClientEntity,
 ):
     """
     Base class for a best effort or segment effort.
@@ -871,13 +858,13 @@ class MetaActivity(strava_model.MetaActivity, BoundClientEntity):
         return self.bound_client.get_activity_comments(self.id)
 
     @lazy_property
-    def zones(self) -> list[BaseActivityZone]:
+    def zones(self) -> list[ActivityZone]:
         """Retrieve a list of zones for an activity.
 
         Returns
         -------
         py:class:`list`
-            A list of :class:`stravalib.model.BaseActivityZone` objects.
+            A list of :class:`stravalib.model.ActivityZone` objects.
         """
 
         assert self.bound_client is not None
@@ -922,8 +909,7 @@ class DetailedActivity(
     """
 
     # field overrides from superclass for type extensions:
-    # TODO: should be SummaryGear - check returns
-    gear: Optional[Gear] = None
+    gear: Optional[SummaryGear] = None
     best_efforts: Optional[list[BestEffort]] = None  # type: ignore[assignment]
     # TODO: returning empty list should be  DetailedSegmentEffort object
     # TODO: test on activity with actual segments
@@ -933,7 +919,6 @@ class DetailedActivity(
     splits_standard: Optional[list[Split]] = None  # type: ignore[assignment]
     # TODO: should be PhotosSummary
     photos: Optional[ActivityPhotoMeta] = None
-    # TODO: check Lap returns against spec
     laps: Optional[list[Lap]] = None
 
     # Added for backward compatibility
@@ -993,39 +978,37 @@ class ClubActivity(strava_model.ClubActivity):
     pass
 
 
-class DistributionBucket(TimedZoneRange):
+class TimedZoneDistribution(strava_model.TimedZoneRange):
     """
     A single distribution bucket object, used for activity zones.
 
     Notes
     -----
-    In this object, we override types for min/max values. The
-    strava API incorrectly types zones as being `int`. However it can
-    return `int` or `float`.
+    Min/max value types are overridden. The
+    Strava API incorrectly types zones as being `int`. However it can
+    return `int` or `float` (floats are returned for pace, ints for heartrate
+    and power).
     """
 
-    # Overrides due to a bug in the Strava API docs
-    # Because the created strava_model.py has this typed as int we will need
-    # to override these types
+    # Type overrides to support pace values returned as ints
     min: Optional[int | float] = None  # type: ignore
     max: Optional[int | float] = None  # type: ignore
 
 
-class BaseActivityZone(
-    ActivityZone,
+class ActivityZone(
+    strava_model.ActivityZone,
     BoundClientEntity,
 ):
     """
     Base class for activity zones.
 
-    A collection of :class:`stravalib.model.DistributionBucket` objects.
+    A collection of :class:`stravalib.strava_model.TimedZoneDistribution` objects.
     """
 
     # Field overrides from superclass for type extensions:
-    # Using type that is currently mimicking legacy behavior.
-    distribution_buckets: Optional[list[DistributionBucket]] = None  # type: ignore[assignment]
+    distribution_buckets: Optional[list[TimedZoneDistribution]] = None  # type: ignore[assignment]
 
-    # TODO: ignoring type given legacy support will be deprecated
+    # strava_model only contains heartrate and power (ints), but also returns pace (float)
     type: Optional[Literal["heartrate", "power", "pace"]] = None  # type: ignore[assignment]
 
 


Test patch:
diff --git a/src/stravalib/tests/integration/test_client.py b/src/stravalib/tests/integration/test_client.py
index 9ae0675..da5420d 100644
--- a/src/stravalib/tests/integration/test_client.py
+++ b/src/stravalib/tests/integration/test_client.py
@@ -9,7 +9,7 @@ from responses import matchers
 
 from stravalib.client import ActivityUploader
 from stravalib.exc import AccessUnauthorized, ActivityPhotoUploadFailed
-from stravalib.model import Athlete, SummaryAthlete, SummarySegment
+from stravalib.model import DetailedAthlete, SummaryAthlete, SummarySegment
 from stravalib.strava_model import SummaryActivity
 from stravalib.tests import RESOURCES_DIR
 from stravalib.unithelper import miles
@@ -52,7 +52,7 @@ def default_request_params():
 def test_get_athlete(mock_strava_api, client):
     mock_strava_api.get("/athlete", response_update={"id": 42})
     athlete = client.get_athlete()
-    assert isinstance(athlete, Athlete)
+    assert isinstance(athlete, DetailedAthlete)
     assert athlete.id == 42
     assert athlete.measurement_preference == "feet"
 
diff --git a/src/stravalib/tests/unit/test_model.py b/src/stravalib/tests/unit/test_model.py
index 5f1eb8b..d799eb2 100644
--- a/src/stravalib/tests/unit/test_model.py
+++ b/src/stravalib/tests/unit/test_model.py
@@ -1,7 +1,6 @@
 from datetime import datetime, timezone
 
 import pytest
-from dateutil.parser import ParserError
 
 from stravalib import model
 from stravalib.model import (
@@ -264,7 +263,7 @@ class ModelTest(TestBase):
                 },
             ]
         }
-        a = model.Athlete.model_validate(d)
+        a = model.DetailedAthlete.model_validate(d)
 
         self.assertEqual(3, len(a.clubs))
         self.assertEqual("Team Roaring Mouse", a.clubs[0].name)
@@ -301,14 +300,21 @@ class ModelTest(TestBase):
 @pytest.mark.parametrize(
     "input_value, expected_output, exception",
     [
+        (0, datetime(1970, 1, 1), None),
         ("2024-04-28T12:00:00Z", datetime(2024, 4, 28, 12, 0), None),
         (
-            int(datetime(2022, 4, 28, 12, 0).timestamp()),
+            int(datetime(2022, 4, 28, 12, 0, tzinfo=timezone.utc).timestamp()),
             datetime(2022, 4, 28, 12, 0),
             None,
         ),
         (
-            str(int(datetime(2022, 4, 28, 12, 0).timestamp())),
+            str(
+                int(
+                    datetime(
+                        2022, 4, 28, 12, 0, tzinfo=timezone.utc
+                    ).timestamp()
+                )
+            ),
             datetime(2022, 4, 28, 12, 0),
             None,
         ),
@@ -322,7 +328,9 @@ class ModelTest(TestBase):
             datetime(2024, 4, 28, 12, 0),
             None,
         ),
-        ("Foo", None, ParserError),
+        ("Foo", None, ValueError),
+        ({"foo": 42}, None, ValueError),
+        (None, None, None),
     ],
 )
 def test_naive_datetime(input_value, expected_output, exception):