Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error messages #1131

Merged
merged 4 commits into from
Feb 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,5 @@ jobs:

- name: Upload coverage report
uses: codecov/codecov-action@v4
with:
token: ${{ secrets.CODECOV_TOKEN }}
7 changes: 5 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions osmnx/_nominatim.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions osmnx/_overpass.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down
6 changes: 3 additions & 3 deletions osmnx/bearing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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):
Expand Down
10 changes: 5 additions & 5 deletions osmnx/distance.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@
# 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])
Expand All @@ -219,7 +219,7 @@
else:
# ensure all coordinates can be converted to float and are non-null
if np.isnan(c.astype(float)).any():
raise ValueError(msg)

Check warning on line 222 in osmnx/distance.py

View check run for this annotation

Codecov / codecov/patch

osmnx/distance.py#L222

Added line #L222 was not covered by tests

# calculate great circle distances, round, and fill nulls with zeros
dists = great_circle(c[:, 0], c[:, 1], c[:, 2], c[:, 3])
Expand Down Expand Up @@ -341,7 +341,7 @@
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"]]
Expand All @@ -351,15 +351,15 @@
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"]])
Expand Down Expand Up @@ -499,7 +499,7 @@
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]
Expand Down
2 changes: 1 addition & 1 deletion osmnx/elevation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions osmnx/features.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)):
Expand Down Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions osmnx/geocoder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions osmnx/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = (
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion osmnx/io.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down
8 changes: 4 additions & 4 deletions osmnx/plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
2 changes: 1 addition & 1 deletion osmnx/projection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions osmnx/routing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion osmnx/simplification.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions osmnx/speed.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions osmnx/stats.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)))

Expand Down Expand Up @@ -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))

Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion osmnx/truncate.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@

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."

Check warning on line 147 in osmnx/truncate.py

View check run for this annotation

Codecov / codecov/patch

osmnx/truncate.py#L147

Added line #L147 was not covered by tests
raise ValueError(msg)

# now identify all nodes whose point geometries lie outside the polygon
Expand Down
4 changes: 2 additions & 2 deletions osmnx/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())
Expand Down
Loading