Enhance logging when stops missing latitude and longitude

Author: davidgamez

functions-python/helpers/requirements.txt

@@ -28,4 +28,5 @@ google-cloud-bigquery
 
 # Additional package
 pycountry
-shapely
+shapely
+pandas

functions-python/helpers/tests/test_transform.py

@@ -1,4 +1,14 @@
-from transform import to_boolean, get_nested_value
+import unittest
+
+import pandas as pd
+
+from transform import (
+    to_boolean,
+    get_nested_value,
+    to_float,
+    get_safe_value,
+    get_safe_float,
+)
 
 
 def test_to_boolean():
@@ -60,3 +70,87 @@ def test_get_nested_value():
     # Test case 9: Non-dictionary data
     assert get_nested_value("not a dict", ["a", "b", "c"]) is None
     assert get_nested_value("not a dict", ["a", "b", "c"], []) == []
+
+
+class TestToFloat(unittest.TestCase):
+    def test_valid_float(self):
+        self.assertEqual(to_float("3.14"), 3.14)
+        self.assertEqual(to_float(2.5), 2.5)
+        self.assertEqual(to_float("0"), 0.0)
+        self.assertEqual(to_float(0), 0.0)
+
+    def test_invalid_float(self):
+        self.assertIsNone(to_float("abc"))
+        self.assertIsNone(to_float(None))
+        self.assertIsNone(to_float(""))
+
+    def test_default_value(self):
+        self.assertEqual(to_float("abc", default_value=1.23), 1.23)
+        self.assertEqual(to_float(None, default_value=4.56), 4.56)
+        self.assertEqual(to_float("", default_value=7.89), 7.89)
+
+
+class TestGetSafeValue(unittest.TestCase):
+    def test_valid_value(self):
+        row = {"name": " Alice "}
+        self.assertEqual(get_safe_value(row, "name"), "Alice")
+
+    def test_missing_column(self):
+        row = {"age": 30}
+        self.assertIsNone(get_safe_value(row, "name"))
+
+    def test_empty_string(self):
+        row = {"name": "   "}
+        self.assertIsNone(get_safe_value(row, "name"))
+
+    def test_nan_value(self):
+        row = {"name": pd.NA}
+        self.assertIsNone(get_safe_value(row, "name"))
+        row = {"name": float("nan")}
+        self.assertIsNone(get_safe_value(row, "name"))
+
+    def test_default_value(self):
+        row = {"name": ""}
+        self.assertEqual(
+            get_safe_value(row, "name", default_value="default"), "default"
+        )
+
+
+class TestGetSafeFloat(unittest.TestCase):
+    def test_valid_float(self):
+        row = {"value": "3.14"}
+        self.assertEqual(get_safe_float(row, "value"), 3.14)
+        row = {"value": 2.5}
+        self.assertEqual(get_safe_float(row, "value"), 2.5)
+        row = {"value": "0"}
+        self.assertEqual(get_safe_float(row, "value"), 0.0)
+        row = {"value": 0}
+        self.assertEqual(get_safe_float(row, "value"), 0.0)
+
+    def test_missing_column(self):
+        row = {"other": 1.23}
+        self.assertIsNone(get_safe_float(row, "value"))
+
+    def test_empty_string(self):
+        row = {"value": "   "}
+        self.assertIsNone(get_safe_float(row, "value"))
+
+    def test_nan_value(self):
+        row = {"value": pd.NA}
+        self.assertIsNone(get_safe_float(row, "value"))
+        row = {"value": float("nan")}
+        self.assertIsNone(get_safe_float(row, "value"))
+
+    def test_invalid_float(self):
+        row = {"value": "abc"}
+        self.assertIsNone(get_safe_float(row, "value"))
+        row = {"value": None}
+        self.assertIsNone(get_safe_float(row, "value"))
+
+    def test_default_value(self):
+        row = {"value": ""}
+        self.assertEqual(get_safe_float(row, "value", default_value=1.23), 1.23)
+        row = {"value": "abc"}
+        self.assertEqual(get_safe_float(row, "value", default_value=4.56), 4.56)
+        row = {"value": None}
+        self.assertEqual(get_safe_float(row, "value", default_value=7.89), 7.89)

functions-python/helpers/transform.py

@@ -96,7 +96,11 @@ def get_safe_value(row, column_name, default_value=None) -> Optional[str]:
     import pandas
 
     value = row.get(column_name, None)
-    if not value or pandas.isna(value) or f"{value}".strip() == "":
+    if (
+        value is None
+        or pandas.isna(value)
+        or (isinstance(value, str) and value.strip() == "")
+    ):
         return default_value
     return f"{value}".strip()
 
@@ -105,12 +109,8 @@ def get_safe_float(row, column_name, default_value=None) -> Optional[float]:
     """
     Get a safe float value from the row. If the value is missing or cannot be converted to float,
     """
-    import pandas
-
-    value = row.get(column_name, None)
-    if not value or pandas.isna(value) or f"{value}".strip() == "":
-        return default_value
+    safe_value = get_safe_value(row, column_name)
     try:
-        return float(value)
+        return float(safe_value)
     except (ValueError, TypeError):
         return default_value

functions-python/pmtiles_builder/src/csv_cache.py

@@ -16,6 +16,7 @@
 import csv
 import os
 
+from gtfs import stop_txt_is_lat_log_required
 from shared.helpers.logger import get_logger
 from shared.helpers.transform import get_safe_value, get_safe_float
 
@@ -134,8 +135,18 @@ def get_coordinates_for_stop(self, stop_id) -> tuple[float, float] | None:
                 row_stop_id = get_safe_value(s, "stop_id")
                 row_stop_lon = get_safe_float(s, "stop_lon")
                 row_stop_lat = get_safe_float(s, "stop_lat")
-                if row_stop_id is None or row_stop_lon is None or row_stop_lat is None:
-                    self.logger.warning("Invalid stop data: %s", s)
+                if row_stop_id is None:
+                    self.logger.warning("Missing stop id: %s", s)
+                    continue
+                if row_stop_lon is None or row_stop_lat is None:
+                    if stop_txt_is_lat_log_required(s):
+                        self.logger.warning(
+                            "Missing stop latitude and longitude : %s", s
+                        )
+                    else:
+                        self.logger.debug(
+                            "Missing optional stop latitude and longitude : %s", s
+                        )
                     continue
                 self.stop_to_coordinates[row_stop_id] = (row_stop_lon, row_stop_lat)
         return self.stop_to_coordinates.get(stop_id, None)

functions-python/pmtiles_builder/src/gtfs.py

@@ -0,0 +1,20 @@
+from shared.helpers.transform import get_safe_value
+
+# TODO: Move this file to a shared folder
+
+
+def stop_txt_is_lat_log_required(stop_row):
+    """
+    Conditionally Required:
+    - Required for locations which are stops (location_type=0), stations (location_type=1)
+    or entrances/exits (location_type=2).
+    - Optional for locations which are generic nodes (location_type=3) or boarding areas (location_type=4).
+
+    Args:
+        row (dict): The data row to check.
+
+    Returns:
+        bool: True if both latitude and longitude is required, False otherwise.
+    """
+    location_type = get_safe_value(stop_row, "location_type", 0)
+    return location_type in ("0", "1", "2")

functions-python/pmtiles_builder/src/main.py

@@ -403,7 +403,11 @@ def _create_routes_geojson(self):
                     if shape_id:
                         coordinates = self._get_shape_points(shape_id, shapes_index)
                     if not coordinates:
-                        # We don't have the coordinates for the shape, fallback on using stops.
+                        logging.info(
+                            "Could not find coordinates for the shape: %s, "
+                            "fallback on using stops.",
+                            shape_id,
+                        )
                         trip_id = self.csv_cache.get_trip_from_route(route_id)
 
                         if trip_id:

functions-python/pmtiles_builder/tests/test_gtfs.py

@@ -0,0 +1,20 @@
+import unittest
+
+from gtfs import stop_txt_is_lat_log_required
+
+
+class TestStopTxtIsLatLogRequired(unittest.TestCase):
+    def test_required_cases(self):
+        self.assertTrue(stop_txt_is_lat_log_required({"location_type": "0"}))
+        self.assertTrue(stop_txt_is_lat_log_required({"location_type": 0}))
+        self.assertTrue(stop_txt_is_lat_log_required({"location_type": "2"}))
+
+    def test_not_required_cases(self):
+        self.assertFalse(stop_txt_is_lat_log_required({"location_type": "3"}))
+        self.assertFalse(stop_txt_is_lat_log_required({"location_type": 3}))
+        self.assertFalse(stop_txt_is_lat_log_required({"location_type": "4"}))
+        self.assertFalse(stop_txt_is_lat_log_required({"location_type": 4}))
+
+    def test_missing_location_type(self):
+        self.assertFalse(stop_txt_is_lat_log_required({}))
+        self.assertFalse(stop_txt_is_lat_log_required({"other": "value"}))