Merge pull request #1131 from gboeing/msg

Improve error messages

.github/workflows/ci.yml

@@ -54,3 +54,5 @@ jobs:
 
       - name: Upload coverage report
         uses: codecov/codecov-action@v4
+        with:
+          token: ${{ secrets.CODECOV_TOKEN }}

CHANGELOG.md

@@ -2,11 +2,14 @@
 
 ## 2.0.0 (in development)
 
+Read the v2 [migration guide](https://github.com/gboeing/osmnx/issues/1123).
+
 - add type annotations to all public and private functions throughout package (#1107)
-- improve docstrings throughout package (#1116)
-- improve logging and warnings throughout package (#1125)
 - remove functionality previously deprecated in v1 (#1113 #1122)
 - drop Python 3.8 support (#1106)
+- improve docstrings throughout package (#1116)
+- improve logging and warnings throughout package (#1125)
+- improve error messages throughout package (#1131)
 - increase add_node_elevations_google default batch_size to 512 to match Google's limit (#1115)
 - make which_result function parameter consistently able to accept a list throughout package (#1113)
 - make utils_geo.bbox_from_point function return a tuple of floats for consistency with rest of package (#1113)

osmnx/_nominatim.py

@@ -47,7 +47,7 @@ def _download_nominatim_element(
     if by_osmid:
         # if querying by OSM ID, use the lookup endpoint
         if not isinstance(query, str):
-            msg = "`query` must be a string if `by_osmid` is True"
+            msg = "`query` must be a string if `by_osmid` is True."
             raise TypeError(msg)
         request_type = "lookup"
         params["osm_ids"] = query
@@ -68,7 +68,7 @@ def _download_nominatim_element(
             for key in sorted(query):
                 params[key] = query[key]
         else:  # pragma: no cover
-            msg = "each query must be a dict or a string"  # type: ignore[unreachable]
+            msg = "Each query must be a dict or a string."  # type: ignore[unreachable]
             raise TypeError(msg)
 
     # request the URL, return the JSON
@@ -102,7 +102,7 @@ def _nominatim_request(
     response_json
     """
     if request_type not in {"search", "reverse", "lookup"}:  # pragma: no cover
-        msg = 'Nominatim request_type must be "search", "reverse", or "lookup"'
+        msg = "Nominatim `request_type` must be 'search', 'reverse', or 'lookup'."
         raise ValueError(msg)
 
     # add nominatim API key to params if one has been provided in settings

osmnx/_overpass.py

@@ -108,7 +108,7 @@ def _get_network_filter(network_type: str) -> str:
     if network_type in filters:
         overpass_filter = filters[network_type]
     else:  # pragma: no cover
-        msg = f"Unrecognized network_type {network_type!r}"
+        msg = f"Unrecognized network_type {network_type!r}."
         raise ValueError(msg)
 
     return overpass_filter
@@ -269,7 +269,7 @@ def _create_overpass_features_query(  # noqa: PLR0912
     overpass_settings = _make_overpass_settings()
 
     # make sure every value in dict is bool, str, or list of str
-    err_msg = "tags must be a dict with values of bool, str, or list of str"
+    err_msg = "`tags` must be a dict with values of bool, str, or list of str."
     if not isinstance(tags, dict):  # pragma: no cover
         raise TypeError(err_msg)
 

osmnx/bearing.py

@@ -105,7 +105,7 @@ def add_edge_bearings(G: nx.MultiDiGraph) -> nx.MultiDiGraph:
         Graph with `bearing` attributes on the edges.
     """
     if projection.is_projected(G.graph["crs"]):  # pragma: no cover
-        msg = "graph must be unprojected to add edge bearings"
+        msg = "Graph must be unprojected to add edge bearings."
         raise ValueError(msg)
 
     # extract edge IDs and corresponding coordinates from their nodes
@@ -161,7 +161,7 @@ def orientation_entropy(
     """
     # check if we were able to import scipy
     if scipy is None:  # pragma: no cover
-        msg = "scipy must be installed as an optional dependency to calculate entropy"
+        msg = "scipy must be installed as an optional dependency to calculate entropy."
         raise ImportError(msg)
     bin_counts, _ = _bearings_distribution(Gu, num_bins, min_length, weight)
     entropy: float = scipy.stats.entropy(bin_counts)
@@ -198,7 +198,7 @@ def _extract_edge_bearings(
         The bidirectional edge bearings of `Gu`.
     """
     if nx.is_directed(Gu) or projection.is_projected(Gu.graph["crs"]):  # pragma: no cover
-        msg = "graph must be undirected and unprojected to analyze edge bearings"
+        msg = "Graph must be undirected and unprojected to analyze edge bearings."
         raise ValueError(msg)
     bearings = []
     for u, v, data in Gu.edges(data=True):

osmnx/distance.py

@@ -210,7 +210,7 @@ def add_edge_lengths(
     # extract edge IDs and corresponding coordinates from their nodes
     x = G.nodes(data="x")
     y = G.nodes(data="y")
-    msg = "some edges missing nodes, possibly due to input data clipping issue"
+    msg = "Some edges missing nodes, possibly due to input data clipping issue."
     try:
         # two-dimensional array of coordinates: y0, x0, y1, x1
         c = np.array([(y[u], x[u], y[v], x[v]) for u, v, k in uvk])
@@ -341,7 +341,7 @@ def nearest_nodes(
         Y_arr = np.array(Y)
 
     if np.isnan(X_arr).any() or np.isnan(Y_arr).any():  # pragma: no cover
-        msg = "`X` and `Y` cannot contain nulls"
+        msg = "`X` and `Y` cannot contain nulls."
         raise ValueError(msg)
 
     nodes = utils_graph.graph_to_gdfs(G, edges=False, node_geometry=False)[["x", "y"]]
@@ -351,15 +351,15 @@ def nearest_nodes(
     if projection.is_projected(G.graph["crs"]):
         # if projected, use k-d tree for euclidean nearest-neighbor search
         if cKDTree is None:  # pragma: no cover
-            msg = "scipy must be installed as an optional dependency to search a projected graph"
+            msg = "scipy must be installed as an optional dependency to search a projected graph."
             raise ImportError(msg)
         dist_array, pos = cKDTree(nodes).query(np.array([X_arr, Y_arr]).T, k=1)
         nn_array = nodes.index[pos].to_numpy()
 
     else:
         # if unprojected, use ball tree for haversine nearest-neighbor search
         if BallTree is None:  # pragma: no cover
-            msg = "scikit-learn must be installed as an optional dependency to search an unprojected graph"
+            msg = "scikit-learn must be installed as an optional dependency to search an unprojected graph."
             raise ImportError(msg)
         # haversine requires lat, lon coords in radians
         nodes_rad = np.deg2rad(nodes[["y", "x"]])
@@ -499,7 +499,7 @@ def nearest_edges(
         Y_arr = np.array(Y)
 
     if np.isnan(X_arr).any() or np.isnan(Y_arr).any():  # pragma: no cover
-        msg = "`X` and `Y` cannot contain nulls"
+        msg = "`X` and `Y` cannot contain nulls."
         raise ValueError(msg)
     geoms = utils_graph.graph_to_gdfs(G, nodes=False)["geometry"]
     ne_array: np.typing.NDArray[np.object_]  # array of tuple[int, int, int]

osmnx/elevation.py

@@ -136,7 +136,7 @@ def add_node_elevations_raster(
         Graph with `elevation` attributes on the nodes.
     """
     if rasterio is None or gdal is None:  # pragma: no cover
-        msg = "gdal and rasterio must be installed as optional dependencies to query raster files"
+        msg = "gdal and rasterio must be installed as optional dependencies to query raster files."
         raise ImportError(msg)
 
     if cpus is None:

osmnx/features.py

@@ -312,7 +312,7 @@ def features_from_polygon(
     """
     # verify that the geometry is valid and is a Polygon/MultiPolygon
     if not polygon.is_valid:
-        msg = "The geometry of `polygon` is invalid"
+        msg = "The geometry of `polygon` is invalid."
         raise ValueError(msg)
 
     if not isinstance(polygon, (Polygon, MultiPolygon)):
@@ -409,7 +409,7 @@ def _create_gdf(  # noqa: PLR0912
             response_count += 1
         msg = f"Retrieved all data from API in {response_count} request(s)"
         utils.log(msg, level=lg.INFO)
-        msg = "Interrupted because `settings.cache_only_mode=True`"
+        msg = "Interrupted because `settings.cache_only_mode=True`."
         raise CacheOnlyInterruptError(msg)
 
     # Dictionaries to hold nodes and complete geometries

osmnx/geocoder.py

@@ -56,7 +56,7 @@ def geocode(query: str) -> tuple[float, float]:
         return point
 
     # otherwise we got no results back
-    msg = f"Nominatim could not geocode query {query!r}"
+    msg = f"Nominatim could not geocode query {query!r}."
     raise InsufficientResponseError(msg)
 
 
@@ -116,7 +116,7 @@ def geocode_to_gdf(
 
     # ensure same length
     if len(q_list) != len(wr_list):  # pragma: no cover
-        msg = "which_result length must equal query length"
+        msg = "`which_result` length must equal `query` length."
         raise ValueError(msg)
 
     # geocode each query, concat as GeoDataFrame rows, then set the CRS
@@ -159,7 +159,7 @@ def _geocode_query_to_gdf(
     # choose the right result from the JSON response
     if len(results) == 0:
         # if no results were returned, raise error
-        msg = f"Nominatim geocoder returned 0 results for query {query!r}"
+        msg = f"Nominatim geocoder returned 0 results for query {query!r}."
         raise InsufficientResponseError(msg)
 
     if by_osmid:
@@ -171,7 +171,7 @@ def _geocode_query_to_gdf(
         try:
             result = _get_first_polygon(results)
         except TypeError as e:
-            msg = f"Nominatim did not geocode query {query!r} to a geometry of type (Multi)Polygon"
+            msg = f"Nominatim did not geocode query {query!r} to a geometry of type (Multi)Polygon."
             raise TypeError(msg) from e
 
     elif len(results) >= which_result:
@@ -180,7 +180,7 @@ def _geocode_query_to_gdf(
 
     else:  # pragma: no cover
         # else, we got fewer results than which_result, raise error
-        msg = f"Nominatim returned {len(results)} result(s) but which_result={which_result}"
+        msg = f"Nominatim returned {len(results)} result(s) but `which_result={which_result}`."
         raise InsufficientResponseError(msg)
 
     # if we got a non (Multi)Polygon geometry type (like a point), log warning

osmnx/graph.py

@@ -165,7 +165,7 @@ def graph_from_point(
     documentation for caveats.
     """
     if dist_type not in {"bbox", "network"}:  # pragma: no cover
-        msg = 'dist_type must be "bbox" or "network"'
+        msg = "`dist_type` must be 'bbox' or 'network'."
         raise ValueError(msg)
 
     # create bounding box from center point and distance in each direction
@@ -403,7 +403,7 @@ def graph_from_polygon(
     # verify that the geometry is valid and is a shapely Polygon/MultiPolygon
     # before proceeding
     if not polygon.is_valid:  # pragma: no cover
-        msg = "The geometry to query within is invalid"
+        msg = "The geometry of `polygon` is invalid."
         raise ValueError(msg)
     if not isinstance(polygon, (Polygon, MultiPolygon)):  # pragma: no cover
         msg = (
@@ -556,7 +556,7 @@ def _create_graph(
     utils.log(msg, level=lg.INFO)
     if settings.cache_only_mode:  # pragma: no cover
         # after consuming all response_jsons in loop, raise exception to catch
-        msg = "Interrupted because `settings.cache_only_mode=True`"
+        msg = "Interrupted because `settings.cache_only_mode=True`."
         raise CacheOnlyInterruptError(msg)
 
     # ensure we got some node/way data back from the server request(s)

osmnx/io.py

@@ -472,7 +472,7 @@ def _convert_bool_string(value: bool | str) -> bool:
         return value == "True"
 
     # otherwise the value is not a valid boolean
-    msg = f"invalid literal for boolean: {value!r}"
+    msg = f"Invalid literal for boolean: {value!r}."
     raise ValueError(msg)
 
 

osmnx/plot.py

@@ -408,13 +408,13 @@ def plot_graph_routes(
 
     # check for valid arguments
     if not all(isinstance(r, list) for r in routes):  # pragma: no cover
-        msg = "`routes` must be an iterable of route lists"
+        msg = "`routes` must be an iterable of route lists."
         raise TypeError(msg)
     if len(routes) == 0:  # pragma: no cover
-        msg = "You must pass at least 1 route"
+        msg = "You must pass at least 1 route."
         raise ValueError(msg)
     if not (len(routes) == len(route_colors) == len(route_linewidths)):  # pragma: no cover
-        msg = "`route_colors` and `route_linewidths` must have same lengths as `routes`"
+        msg = "`route_colors` and `route_linewidths` must have same lengths as `routes`."
         raise ValueError(msg)
 
     # plot the graph and the first route
@@ -1047,5 +1047,5 @@ def _verify_mpl() -> None:
     None
     """
     if not mpl_available:  # pragma: no cover
-        msg = "matplotlib must be installed as an optional dependency for visualization"
+        msg = "matplotlib must be installed as an optional dependency for visualization."
         raise ImportError(msg)

osmnx/projection.py

@@ -105,7 +105,7 @@ def project_gdf(
         The projected GeoDataFrame.
     """
     if gdf.crs is None or len(gdf) == 0:  # pragma: no cover
-        msg = "GeoDataFrame must have a valid CRS and cannot be empty"
+        msg = "`gdf` must have a valid CRS and cannot be empty."
         raise ValueError(msg)
 
     # if to_latlong is True, project the gdf to the default_crs

osmnx/routing.py

@@ -156,15 +156,15 @@ def shortest_path(
 
     # if only 1 of orig or dest is iterable and the other is not, raise error
     if not (isinstance(orig, Iterable) and isinstance(dest, Iterable)):
-        msg = "orig and dest must either both be iterable or neither must be iterable"
+        msg = "`orig` and `dest` must either both be iterable or neither must be iterable."
         raise TypeError(msg)
 
     # if both orig and dest are iterable, make them lists (so we're guaranteed
     # to be able to get their sizes) then ensure they have same lengths
     orig = list(orig)
     dest = list(dest)
     if len(orig) != len(dest):  # pragma: no cover
-        msg = "orig and dest must be of equal length"
+        msg = "`orig` and `dest` must be of equal length."
         raise ValueError(msg)
 
     # determine how many cpu cores to use

osmnx/simplification.py

@@ -163,7 +163,7 @@ def _build_path(
                     # if successor has >1 successors, then successor must have
                     # been an endpoint because you can go in 2 new directions.
                     # this should never occur in practice
-                    msg = f"Impossible simplify pattern failed near {successor}"
+                    msg = f"Impossible simplify pattern failed near {successor}."
                     raise GraphSimplificationError(msg)
 
             # if this successor is an endpoint, we've completed the path

osmnx/speed.py

@@ -111,7 +111,7 @@ def add_edge_speeds(
     # caller did not pass in hwy_speeds or fallback arguments
     if pd.isna(speed_kph).all():
         msg = (
-            "this graph's edges have no preexisting `maxspeed` attribute "
+            "This graph's edges have no preexisting 'maxspeed' attribute "
             "values so you must pass `hwy_speeds` or `fallback` arguments."
         )
         raise ValueError(msg)
@@ -146,12 +146,12 @@ def add_edge_travel_times(G: nx.MultiDiGraph) -> nx.MultiDiGraph:
 
     # verify edge length and speed_kph attributes exist
     if not ("length" in edges.columns and "speed_kph" in edges.columns):  # pragma: no cover
-        msg = "all edges must have `length` and `speed_kph` attributes."
+        msg = "All edges must have 'length' and 'speed_kph' attributes."
         raise KeyError(msg)
 
     # verify edge length and speed_kph attributes contain no nulls
     if pd.isna(edges["length"]).any() or pd.isna(edges["speed_kph"]).any():  # pragma: no cover
-        msg = "edge `length` and `speed_kph` values must be non-null."
+        msg = "Edge 'length' and 'speed_kph' values must be non-null."
         raise ValueError(msg)
 
     # convert distance meters to km, and speed km per hour to km per second

osmnx/stats.py

@@ -156,7 +156,7 @@ def street_segment_count(Gu: nx.MultiGraph) -> int:
         Count of street segments in graph.
     """
     if nx.is_directed(Gu):  # pragma: no cover
-        msg = "`Gu` must be undirected"
+        msg = "`Gu` must be undirected."
         raise ValueError(msg)
     return len(Gu.edges)
 
@@ -176,7 +176,7 @@ def street_length_total(Gu: nx.MultiGraph) -> float:
         Total length (meters) of streets in graph.
     """
     if nx.is_directed(Gu):  # pragma: no cover
-        msg = "`Gu` must be undirected"
+        msg = "`Gu` must be undirected."
         raise ValueError(msg)
     return float(sum(d["length"] for u, v, d in Gu.edges(data=True)))
 
@@ -215,7 +215,7 @@ def self_loop_proportion(Gu: nx.MultiGraph) -> float:
         Proportion of graph edges that are self-loops.
     """
     if nx.is_directed(Gu):  # pragma: no cover
-        msg = "`Gu` must be undirected"
+        msg = "`Gu` must be undirected."
         raise ValueError(msg)
     return float(sum(u == v for u, v, k in Gu.edges) / len(Gu.edges))
 
@@ -240,7 +240,7 @@ def circuity_avg(Gu: nx.MultiGraph) -> float | None:
         The graph's average undirected edge circuity.
     """
     if nx.is_directed(Gu):  # pragma: no cover
-        msg = "`Gu` must be undirected"
+        msg = "`Gu` must be undirected."
         raise ValueError(msg)
 
     # extract the edges' endpoint nodes' coordinates

osmnx/truncate.py

@@ -144,7 +144,7 @@ def truncate_graph_polygon(
 
     if len(to_keep) == 0:
         # no graph nodes within the polygon: can't create a graph from that
-        msg = "Found no graph nodes within the requested polygon"
+        msg = "Found no graph nodes within the requested polygon."
         raise ValueError(msg)
 
     # now identify all nodes whose point geometries lie outside the polygon

osmnx/utils.py

@@ -61,7 +61,7 @@ def citation(style: str = "bibtex") -> None:
             "doi: 10.1016/j.compenvurbsys.2017.05.004."
         )
     else:  # pragma: no cover
-        err_msg = f"unrecognized citation style {style!r}"
+        err_msg = f"Invalid citation style {style!r}."
         raise ValueError(err_msg)
 
     print(msg)  # noqa: T201
@@ -92,7 +92,7 @@ def ts(style: str = "datetime", template: str | None = None) -> str:
         elif style == "time":
             template = "{:%H:%M:%S}"
         else:  # pragma: no cover
-            msg = f"unrecognized timestamp style {style!r}"
+            msg = f"Invalid timestamp style {style!r}."
             raise ValueError(msg)
 
     return template.format(dt.datetime.now().astimezone())