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

Nicer error reporting when geocoder fails #55

Open
cholmes opened this issue Nov 20, 2023 · 6 comments
Open

Nicer error reporting when geocoder fails #55

cholmes opened this issue Nov 20, 2023 · 6 comments
Labels
get_buildings Issues related to the get_buildings operations good first issue Good for newcomers
Milestone

Comments

@cholmes
Copy link
Collaborator

cholmes commented Nov 20, 2023

When I request a location with --location and it doesn't have results I get a barfed stack trace:

Traceback (most recent call last):
  File "/opt/homebrew/Caskroom/miniforge/base/envs/qgis/bin/ob", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/opt/homebrew/Caskroom/miniforge/base/envs/qgis/lib/python3.11/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Caskroom/miniforge/base/envs/qgis/lib/python3.11/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Caskroom/miniforge/base/envs/qgis/lib/python3.11/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Caskroom/miniforge/base/envs/qgis/lib/python3.11/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Caskroom/miniforge/base/envs/qgis/lib/python3.11/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Caskroom/miniforge/base/envs/qgis/lib/python3.11/site-packages/open_buildings/cli.py", line 84, in get_buildings
    geojson_data = geocode(location)
                   ^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Caskroom/miniforge/base/envs/qgis/lib/python3.11/site-packages/open_buildings/cli.py", line 40, in geocode
    location = osmnx.geocode_to_gdf(data)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Caskroom/miniforge/base/envs/qgis/lib/python3.11/site-packages/osmnx/geocoder.py", line 137, in geocode_to_gdf
    gdf = pd.concat([gdf, _geocode_query_to_gdf(q, wr, by_osmid)])
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Caskroom/miniforge/base/envs/qgis/lib/python3.11/site-packages/osmnx/geocoder.py", line 187, in _geocode_query_to_gdf
    raise InsufficientResponseError(msg)
osmnx._errors.InsufficientResponseError: Nominatim geocoder returned 0 results for query 'adsfsad'

It'd be better to catch the error and just inform users that their location string didn't work - they can get the geojson on their own or try a more common string.

@cholmes cholmes added good first issue Good for newcomers get_buildings Issues related to the get_buildings operations labels Nov 20, 2023
@cholmes cholmes added this to the 0.11.0 milestone Nov 20, 2023
@mtravis
Copy link
Contributor

mtravis commented Nov 26, 2023

I tried a simple fix for this

def geocode(data: str):
    try:
        location = ox.geocode_to_gdf(data)
        geom = location.geometry[0]
        geojson = json.loads(json.dumps({"type": "Feature", "geometry": mapping(geom)})) # turn geom tuple into list by (de-)serialising
        return geojson
    except InsufficientResponseError as e: 
         print(f"An error occurred: {e}")

but getting issues with the geojson_to_quadkey function in download_buildings

  File "/usr/local/lib/python3.10/dist-packages/open_buildings-0.10.0-py3.10.egg/open_buildings/download_buildings.py", line 23, in geojson_to_quadkey
    geom = shape(data["geometry"])
TypeError: 'NoneType' object is not subscriptable

@felix-schott can you try reproducing please?

@felix-schott
Copy link
Collaborator

Hi @mtravis - not on my laptop at the moment, so can't reproduce but I'm guessing it might be because the programme is not exiting and because the geocoding is unsuccessful, the programme continues and expects a geometry instead of None where it raises the error. try-except is the right approach to suppress the long error message, but we need to exit the programme immediately afterwards. A similar thing happens here - maybe copy that approach? The empty return terminates the programme. (On Linux, it would be better to use sys.exit(1) instead of an empty return but I'm not sure how that behaves on other platforms, so just use the empty return.)

@felix-schott
Copy link
Collaborator

I'd also print a more meaningful error message: "No results found for {location} - please try a different search term." Otherwise users don't know what's happening.

@felix-schott
Copy link
Collaborator

error

Actually, sorry, you will have to use sys.exit or raise an exception since return will only exit the helper function, not the whole programme. It might be cleaner to return None from the geocoding function if no result could be found, and then use the empty return approach in the calling function if geocoding_result is None. Not sure if that's clear, if I have time, I can give you a code snippet

@mtravis
Copy link
Contributor

mtravis commented Nov 26, 2023

@felix-schott

I've got it working with sys.exit but will try implementing the return None and updating the calling function later.

My error message is sensible looking once it calls the internal message from the event.

@mtravis
Copy link
Contributor

mtravis commented Nov 26, 2023

@felix-schott I'm struggling a bit with to get the script to exit fully without sys.exit(1). I agree it would be cleaner not to use this method but can't get download_buildings to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
get_buildings Issues related to the get_buildings operations good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants