make transformers work on geo.Path

5 files changed, 77 insertions, 73 deletions

diff --git a/fietsboek/actions.py b/fietsboek/actions.py
index f49283d..ea19f63 100644
--- a/fietsboek/actions.py
+++ b/fietsboek/actions.py
@@ -91,6 +91,7 @@ def add_track(
     # Save the GPX data
     LOGGER.debug("Creating a new data folder for %d", track.id)
     assert track.id is not None
+    path = track.path()
     with data_manager.initialize(track.id) as manager:
         LOGGER.debug("Saving backup to %s", manager.backup_path())
         manager.compress_backup(gpx_data)
@@ -98,11 +99,13 @@ def add_track(
         gpx = gpxpy.parse(track.gpx_xml())
         for transformer in transformers:
             LOGGER.debug("Running %s with %r", transformer, transformer.parameters)
-            transformer.execute(gpx)
+            transformer.execute(path)
         track.transformers = [
             [tfm.identifier(), tfm.parameters.model_dump()] for tfm in transformers
         ]
 
+        track.set_path(path)
+
         # Best time to build the cache is right after the upload, but *after* the
         # transformers have been applied!
         track.ensure_cache()
@@ -188,7 +191,7 @@ def edit_images(request: Request, track: models.Track, *, manager: Optional[Trac
         request.dbsession.add(image_meta)
 
 
-def execute_transformers(request: Request, track: models.Track) -> Optional[gpxpy.gpx.GPX]:
+def execute_transformers(request: Request, track: models.Track):
     """Execute the transformers for the given track.
 
     Note that this function "short circuits" if the saved transformer settings
@@ -199,7 +202,6 @@ def execute_transformers(request: Request, track: models.Track) -> Optional[gpxp
 
     :param request: The request.
     :param track: The track.
-    :return: The transformed track.
     """
     # pylint: disable=too-many-locals
     LOGGER.debug("Executing transformers for %d", track.id)
@@ -209,22 +211,21 @@ def execute_transformers(request: Request, track: models.Track) -> Optional[gpxp
     serialized = [[tfm.identifier(), tfm.parameters.model_dump()] for tfm in settings]
     if serialized == track.transformers:
         LOGGER.debug("Applied transformations match on %d, skipping", track.id)
-        return None
+        return
 
     # We always start with the backup, that way we don't get "deepfried GPX"
     # files by having the same filters run multiple times on the same input.
     # They are not idempotent after all.
     manager = request.data_manager.open(track.id)
-    gpx_bytes = manager.backup_path().read_bytes()
-    gpx_bytes = brotli.decompress(gpx_bytes)
-    gpx = gpxpy.parse(gpx_bytes)
+    backup_bytes = manager.decompress_backup()
+    reloaded = convert.smart_convert(backup_bytes)
+    path = reloaded.path()
 
     for transformer in settings:
         LOGGER.debug("Running %s with %r", transformer, transformer.parameters)
-        transformer.execute(gpx)
+        transformer.execute(path)
 
-    LOGGER.debug("Saving transformed file for %d", track.id)
-    manager.compress_gpx(util.encode_gpx(gpx))
+    track.set_path(path)
 
     LOGGER.debug("Saving new transformers on %d", track.id)
     track.transformers = serialized
@@ -232,9 +233,8 @@ def execute_transformers(request: Request, track: models.Track) -> Optional[gpxp
     LOGGER.debug("Rebuilding cache for %d", track.id)
     request.dbsession.delete(track.cache)
     track.cache = None
-    track.ensure_cache(gpx)
+    track.ensure_cache()
     request.dbsession.add(track.cache)
-    return gpx
 
 
 def send_verification_token(request: Request, user: models.User):
diff --git a/fietsboek/geo.py b/fietsboek/geo.py
index 348a4b9..8b016c0 100644
--- a/fietsboek/geo.py
+++ b/fietsboek/geo.py
@@ -79,6 +79,39 @@ class Point:
             return 0.0
         return sqrt(radicand)
 
+    def flat_distance(self, other: "Point") -> float:
+        """Returns the distance between this point and the other point in
+        meters.
+
+        This does not take elevation into account, and only looks at the 2d distance.
+        """
+        r = EARTH_RADIUS
+        # The formula assumes that 0° is straight upward, but 0° in geo
+        # coordinates is actually on the equator plane.
+        t_1 = radians(90 - self.latitude)
+        t_2 = radians(90 - other.latitude)
+        p_1 = radians(self.longitude)
+        p_2 = radians(other.longitude)
+        # See
+        # https://en.wikipedia.org/wiki/Spherical_coordinate_system#Distance_in_spherical_coordinates
+        # While this is not the Haversine formula for distances along the
+        # circle curvature, it allows us to take the elevation into account,
+        # and for most GPS point differences that we encounter it should be
+        # enough.
+        radicand = (
+            2 * r**2 * (
+                1 -
+                (
+                    sin(t_1) * sin(t_2) * cos(p_1 - p_2) +
+                    cos(t_1) * cos(t_2)
+                )
+            )
+        )
+        if radicand < 0.0:
+            return 0.0
+        return sqrt(radicand)
+
+
 
 class Path:
     def __init__(self, points: list[Point]):
@@ -93,7 +126,10 @@ class Path:
         for a, b in self._point_pairs():
             distance = a.distance(b)
             time = b.time_offset - a.time_offset
-            speed = distance / time
+            if time != 0:
+                speed = distance / time
+            else:
+                speed = 0.0
             elevation = b.elevation - a.elevation
 
             movement_data.length += distance
diff --git a/fietsboek/transformers/__init__.py b/fietsboek/transformers/__init__.py
index b1a0245..d9c533b 100644
--- a/fietsboek/transformers/__init__.py
+++ b/fietsboek/transformers/__init__.py
@@ -18,6 +18,8 @@ from pydantic import BaseModel
 from pyramid.i18n import TranslationString
 from pyramid.request import Request
 
+from .. import geo
+
 _ = TranslationString
 
 T = TypeVar("T", bound="Transformer")
@@ -117,12 +119,12 @@ class Transformer(ABC):
         pass
 
     @abstractmethod
-    def execute(self, gpx: GPX):
+    def execute(self, path: geo.Path):
         """Run the transformation on the input gpx.
 
-        This is expected to modify the GPX object to represent the new state.
+        This is expected to modify the path to represent the new state.
 
-        :param gpx: The GPX object to transform. Note that this object will be
+        :param path: The path to transform. Note that this object will be
             mutated!
         """
 
diff --git a/fietsboek/transformers/breaks.py b/fietsboek/transformers/breaks.py
index 789fdfd..e8af6de 100644
--- a/fietsboek/transformers/breaks.py
+++ b/fietsboek/transformers/breaks.py
@@ -6,6 +6,7 @@ from gpxpy.gpx import GPX, GPXTrack
 from pyramid.i18n import TranslationString
 
 from . import Parameters, Transformer
+from .. import geo
 
 _ = TranslationString
 
@@ -47,34 +48,25 @@ class RemoveBreaks(Transformer):
     def parameters(self, value):
         pass
 
-    def execute(self, gpx: GPX):
-        for track in gpx.tracks:
-            self._clean(track)
-
-    def _clean(self, track: GPXTrack):
-        if not track.get_points_no():
+    def execute(self, path: geo.Path):
+        if not path.points:
             return
 
         i = 0
-        while i < track.get_points_no():
-            segment_idx, point_idx = index(track, i)
-            point = track.segments[segment_idx].points[point_idx]
+        while i < len(path.points):
+            point = path.points[i]
 
             # We check if the following points constitute a break, and if yes,
             # how many of them
             count = 0
             current_length = 0.0
             last_point = point
-            while True:
-                try:
-                    j_segment, j_point = index(track, i + count + 1)
-                except IndexError:
-                    break
-                current_point = track.segments[j_segment].points[j_point]
-                current_length += last_point.distance_3d(current_point) or 0.0
+            while i + count + 1 < len(path.points):
+                current_point = path.points[i + count + 1]
+                current_length += last_point.distance(current_point) or 0.0
                 last_point = current_point
 
-                delta_t = datetime.timedelta(seconds=point.time_difference(last_point) or 0.0)
+                delta_t = datetime.timedelta(seconds=last_point.time_offset - point.time_offset or 0.0)
                 if not delta_t or current_length / delta_t.total_seconds() > STOPPED_SPEED_LIMIT:
                     break
                 count += 1
@@ -85,7 +77,7 @@ class RemoveBreaks(Transformer):
                 continue
 
             # At this point, check if the break is long enough to be removed
-            delta_t = datetime.timedelta(seconds=point.time_difference(last_point) or 0.0)
+            delta_t = datetime.timedelta(seconds=last_point.time_offset - point.time_offset or 0.0)
             if delta_t < MIN_BREAK_TO_REMOVE:
                 i += 1
                 continue
@@ -93,32 +85,12 @@ class RemoveBreaks(Transformer):
             # Here, we have a proper break to remove
             # Delete the points belonging to the break ...
             for _ in range(count):
-                j_segment, j_point = index(track, i + 1)
-                del track.segments[j_segment].points[j_point]
+                del path.points[i + 1]
 
             # ... and shift the time of the following points
             j = i + 1
-            while j < track.get_points_no():
-                j_segment, j_point = index(track, j)
-                track.segments[j_segment].points[j_point].adjust_time(-delta_t)
-                j += 1
-
-
-def index(track: GPXTrack, idx: int) -> tuple[int, int]:
-    """Takes a one-dimensional index (the point index) and returns an index
-    into the segment/segment points.
-
-    :raises IndexError: When the given index is out of bounds.
-    :param track: The track for which to get the index.
-    :param idx: The "1D" index.
-    :return: A tuple with the segment index, and the index of the point within
-    the segment.
-    """
-    for segment_idx, segment in enumerate(track.segments):
-        if idx < len(segment.points):
-            return (segment_idx, idx)
-        idx -= len(segment.points)
-    raise IndexError
+            for j_point in path.points[j:]:
+                j_point.time_offset -= delta_t.total_seconds()
 
 
 __all__ = ["RemoveBreaks"]
diff --git a/fietsboek/transformers/elevation.py b/fietsboek/transformers/elevation.py
index e1f7c7c..52e6d6f 100644
--- a/fietsboek/transformers/elevation.py
+++ b/fietsboek/transformers/elevation.py
@@ -7,13 +7,14 @@ from gpxpy.gpx import GPX, GPXTrackPoint
 from pyramid.i18n import TranslationString
 
 from . import Parameters, Transformer
+from .. import geo
 
 _ = TranslationString
 
 MAX_ORGANIC_SLOPE: float = 1.0
 
 
-def slope(point_a: GPXTrackPoint, point_b: GPXTrackPoint) -> float:
+def slope(point_a: geo.Point, point_b: geo.Point) -> float:
     """Returns the slope between two GPX points.
 
     This is defined as delta_h / euclid_distance.
@@ -25,7 +26,7 @@ def slope(point_a: GPXTrackPoint, point_b: GPXTrackPoint) -> float:
     if point_a.elevation is None or point_b.elevation is None:
         return 0.0
     delta_h = abs(point_a.elevation - point_b.elevation)
-    dist = point_a.distance_2d(point_b)
+    dist = point_a.flat_distance(point_b)
     if dist == 0.0 or dist is None:
         return 0.0
     return delta_h / dist
@@ -58,19 +59,12 @@ class FixNullElevation(Transformer):
     def parameters(self, value):
         pass
 
-    def execute(self, gpx: GPX):
+    def execute(self, path: geo.Path):
         def all_points():
-            return gpx.walk(only_points=True)
+            return iter(path.points)
 
         def rev_points():
-            # We cannot use reversed(gpx.walk(...)) since that is not a
-            # generator, so we do it manually.
-            return (
-                point
-                for track in reversed(gpx.tracks)
-                for segment in reversed(track.segments)
-                for point in reversed(segment.points)
-            )
+            return reversed(path.points)
 
         # First, from the front
         self.fixup(all_points)
@@ -78,7 +72,7 @@ class FixNullElevation(Transformer):
         self.fixup(rev_points)
 
     @classmethod
-    def fixup(cls, points: Callable[[], Iterable[GPXTrackPoint]]):
+    def fixup(cls, points: Callable[[], Iterable[geo.Point]]):
         """Fixes the given GPX points.
 
         This iterates over the points and checks for the first point that has a
@@ -131,11 +125,11 @@ class FixElevationJumps(Transformer):
     def parameters(self, value):
         pass
 
-    def execute(self, gpx: GPX):
+    def execute(self, path: geo.Path):
         current_adjustment = 0.0
 
-        points = gpx.walk(only_points=True)
-        next_points = gpx.walk(only_points=True)
+        points = iter(path.points)
+        next_points = iter(path.points)
 
         for current_point, next_point in zip_longest(points, islice(next_points, 1, None)):
             point_adjustment = current_adjustment