From 6c67a4b500cf08b3cfce96e74b2658deb4696c05 Mon Sep 17 00:00:00 2001 From: Sarah Hoffmann Date: Sun, 26 Mar 2023 18:09:33 +0200 Subject: [PATCH 1/1] switch reverse CLI command to Python implementation --- nominatim/api/reverse.py | 13 ++++--- nominatim/api/v1/server_glue.py | 3 +- nominatim/clicmd/api.py | 61 ++++++++++++++++++++------------ nominatim/clicmd/args.py | 45 ++++++++++++++++++++++++ test/python/cli/test_cmd_api.py | 62 +++++++++++++++++++++++++++++++-- 5 files changed, 152 insertions(+), 32 deletions(-) diff --git a/nominatim/api/reverse.py b/nominatim/api/reverse.py index 60b24fdc..ef6d1041 100644 --- a/nominatim/api/reverse.py +++ b/nominatim/api/reverse.py @@ -30,8 +30,15 @@ def _select_from_placex(t: SaFromClause, wkt: Optional[str] = None) -> SaSelect: """ if wkt is None: distance = t.c.distance + centroid = t.c.centroid else: distance = t.c.geometry.ST_Distance(wkt) + centroid = sa.case( + (t.c.geometry.ST_GeometryType().in_(('ST_LineString', + 'ST_MultiLineString')), + t.c.geometry.ST_ClosestPoint(wkt)), + else_=t.c.centroid).label('centroid') + return sa.select(t.c.place_id, t.c.osm_type, t.c.osm_id, t.c.name, t.c.class_, t.c.type, @@ -39,11 +46,7 @@ def _select_from_placex(t: SaFromClause, wkt: Optional[str] = None) -> SaSelect: t.c.housenumber, t.c.postcode, t.c.country_code, t.c.importance, t.c.wikipedia, t.c.parent_place_id, t.c.rank_address, t.c.rank_search, - sa.case( - (t.c.geometry.ST_GeometryType().in_(('ST_LineString', - 'ST_MultiLineString')), - t.c.geometry.ST_ClosestPoint(wkt)), - else_=t.c.centroid).label('centroid'), + centroid, distance.label('distance'), t.c.geometry.ST_Expand(0).label('bbox')) diff --git a/nominatim/api/v1/server_glue.py b/nominatim/api/v1/server_glue.py index b5a4a3ca..a87b6825 100644 --- a/nominatim/api/v1/server_glue.py +++ b/nominatim/api/v1/server_glue.py @@ -325,8 +325,7 @@ async def reverse_endpoint(api: napi.NominatimAPIAsync, params: ASGIAdaptor) -> fmt_options = {'locales': locales, 'extratags': params.get_bool('extratags', False), 'namedetails': params.get_bool('namedetails', False), - 'addressdetails': params.get_bool('addressdetails', True), - 'single_result': True} + 'addressdetails': params.get_bool('addressdetails', True)} if fmt == 'xml': fmt_options['xml_roottag'] = 'reversegeocode' fmt_options['xml_extra_info'] = {'querystring': 'TODO'} diff --git a/nominatim/clicmd/api.py b/nominatim/clicmd/api.py index a59002a9..41256b79 100644 --- a/nominatim/clicmd/api.py +++ b/nominatim/clicmd/api.py @@ -18,6 +18,7 @@ from nominatim.errors import UsageError from nominatim.clicmd.args import NominatimArgs import nominatim.api as napi import nominatim.api.v1 as api_output +from nominatim.api.v1.server_glue import REVERSE_MAX_RANKS # Do not repeat documentation of subcommand classes. # pylint: disable=C0111 @@ -53,7 +54,8 @@ def _add_api_output_arguments(parser: argparse.ArgumentParser) -> None: group.add_argument('--polygon-output', choices=['geojson', 'kml', 'svg', 'text'], help='Output geometry of results as a GeoJSON, KML, SVG or WKT') - group.add_argument('--polygon-threshold', type=float, metavar='TOLERANCE', + group.add_argument('--polygon-threshold', type=float, default = 0.0, + metavar='TOLERANCE', help=("Simplify output geometry." "Parameter is difference tolerance in degrees.")) @@ -150,26 +152,46 @@ class APIReverse: help='Longitude of coordinate to look up (in WGS84)') group.add_argument('--zoom', type=int, help='Level of detail required for the address') + group.add_argument('--layer', metavar='LAYER', + choices=[n.name.lower() for n in napi.DataLayer if n.name], + action='append', required=False, dest='layers', + help='OSM id to lookup in format (may be repeated)') _add_api_output_arguments(parser) def run(self, args: NominatimArgs) -> int: - params = dict(lat=args.lat, lon=args.lon, format=args.format) - if args.zoom is not None: - params['zoom'] = args.zoom + api = napi.NominatimAPI(args.project_dir) - for param, _ in EXTRADATA_PARAMS: - if getattr(args, param): - params[param] = '1' - if args.lang: - params['accept-language'] = args.lang - if args.polygon_output: - params['polygon_' + args.polygon_output] = '1' - if args.polygon_threshold: - params['polygon_threshold'] = args.polygon_threshold + details = napi.LookupDetails(address_details=True, # needed for display name + geometry_output=args.get_geometry_output(), + geometry_simplification=args.polygon_threshold or 0.0) + + result = api.reverse(napi.Point(args.lon, args.lat), + REVERSE_MAX_RANKS[max(0, min(18, args.zoom or 18))], + args.get_layers(napi.DataLayer.ADDRESS | napi.DataLayer.POI), + details) + + if result: + output = api_output.format_result( + napi.ReverseResults([result]), + args.format, + {'locales': args.get_locales(api.config.DEFAULT_LANGUAGE), + 'extratags': args.extratags, + 'namedetails': args.namedetails, + 'addressdetails': args.addressdetails}) + if args.format != 'xml': + # reformat the result, so it is pretty-printed + json.dump(json.loads(output), sys.stdout, indent=4, ensure_ascii=False) + else: + sys.stdout.write(output) + sys.stdout.write('\n') + + return 0 + + LOG.error("Unable to geocode.") + return 42 - return _run_api('reverse', args, params) class APILookup: @@ -270,23 +292,16 @@ class APIDetails: if args.polygon_geojson: details.geometry_output = napi.GeometryFormat.GEOJSON - if args.lang: - locales = napi.Locales.from_accept_languages(args.lang) - elif api.config.DEFAULT_LANGUAGE: - locales = napi.Locales.from_accept_languages(api.config.DEFAULT_LANGUAGE) - else: - locales = napi.Locales() - result = api.lookup(place, details) if result: output = api_output.format_result( result, 'json', - {'locales': locales, + {'locales': args.get_locales(api.config.DEFAULT_LANGUAGE), 'group_hierarchy': args.group_hierarchy}) # reformat the result, so it is pretty-printed - json.dump(json.loads(output), sys.stdout, indent=4) + json.dump(json.loads(output), sys.stdout, indent=4, ensure_ascii=False) sys.stdout.write('\n') return 0 diff --git a/nominatim/clicmd/args.py b/nominatim/clicmd/args.py index 9be20b20..bf3109ac 100644 --- a/nominatim/clicmd/args.py +++ b/nominatim/clicmd/args.py @@ -10,11 +10,13 @@ Provides custom functions over command-line arguments. from typing import Optional, List, Dict, Any, Sequence, Tuple import argparse import logging +from functools import reduce from pathlib import Path from nominatim.errors import UsageError from nominatim.config import Configuration from nominatim.typing import Protocol +import nominatim.api as napi LOG = logging.getLogger() @@ -162,6 +164,7 @@ class NominatimArgs: lat: float lon: float zoom: Optional[int] + layers: Optional[Sequence[str]] # Arguments to 'lookup' ids: Sequence[str] @@ -211,3 +214,45 @@ class NominatimArgs: raise UsageError('Cannot access file.') return files + + + def get_geometry_output(self) -> napi.GeometryFormat: + """ Get the requested geometry output format in a API-compatible + format. + """ + if not self.polygon_output: + return napi.GeometryFormat.NONE + if self.polygon_output == 'geojson': + return napi.GeometryFormat.GEOJSON + if self.polygon_output == 'kml': + return napi.GeometryFormat.KML + if self.polygon_output == 'svg': + return napi.GeometryFormat.SVG + if self.polygon_output == 'text': + return napi.GeometryFormat.TEXT + + try: + return napi.GeometryFormat[self.polygon_output.upper()] + except KeyError as exp: + raise UsageError(f"Unknown polygon output format '{self.polygon_output}'.") from exp + + + def get_locales(self, default: Optional[str]) -> napi.Locales: + """ Get the locales from the language parameter. + """ + if self.lang: + return napi.Locales.from_accept_languages(self.lang) + if default: + return napi.Locales.from_accept_languages(default) + + return napi.Locales() + + + def get_layers(self, default: napi.DataLayer) -> Optional[napi.DataLayer]: + """ Get the list of selected layers as a DataLayer enum. + """ + if not self.layers: + return default + + return reduce(napi.DataLayer.__or__, + (napi.DataLayer[s.upper()] for s in self.layers)) diff --git a/test/python/cli/test_cmd_api.py b/test/python/cli/test_cmd_api.py index 6ca96827..cff83cef 100644 --- a/test/python/cli/test_cmd_api.py +++ b/test/python/cli/test_cmd_api.py @@ -24,7 +24,6 @@ def test_no_api_without_phpcgi(endpoint): @pytest.mark.parametrize("params", [('search', '--query', 'new'), ('search', '--city', 'Berlin'), - ('reverse', '--lat', '0', '--lon', '0', '--zoom', '13'), ('lookup', '--id', 'N1')]) class TestCliApiCallPhp: @@ -98,6 +97,65 @@ class TestCliDetailsCall: json.loads(capsys.readouterr().out) +class TestCliReverseCall: + + @pytest.fixture(autouse=True) + def setup_reverse_mock(self, monkeypatch): + result = napi.ReverseResult(napi.SourceTable.PLACEX, ('place', 'thing'), + napi.Point(1.0, -3.0), + names={'name':'Name', 'name:fr': 'Nom'}, + extratags={'extra':'Extra'}) + + monkeypatch.setattr(napi.NominatimAPI, 'reverse', + lambda *args: result) + + + def test_reverse_simple(self, cli_call, tmp_path, capsys): + result = cli_call('reverse', '--project-dir', str(tmp_path), + '--lat', '34', '--lon', '34') + + assert result == 0 + + out = json.loads(capsys.readouterr().out) + assert out['name'] == 'Name' + assert 'address' not in out + assert 'extratags' not in out + assert 'namedetails' not in out + + + @pytest.mark.parametrize('param,field', [('--addressdetails', 'address'), + ('--extratags', 'extratags'), + ('--namedetails', 'namedetails')]) + def test_reverse_extra_stuff(self, cli_call, tmp_path, capsys, param, field): + result = cli_call('reverse', '--project-dir', str(tmp_path), + '--lat', '34', '--lon', '34', param) + + assert result == 0 + + out = json.loads(capsys.readouterr().out) + assert field in out + + + def test_reverse_format(self, cli_call, tmp_path, capsys): + result = cli_call('reverse', '--project-dir', str(tmp_path), + '--lat', '34', '--lon', '34', '--format', 'geojson') + + assert result == 0 + + out = json.loads(capsys.readouterr().out) + assert out['type'] == 'FeatureCollection' + + + def test_reverse_language(self, cli_call, tmp_path, capsys): + result = cli_call('reverse', '--project-dir', str(tmp_path), + '--lat', '34', '--lon', '34', '--lang', 'fr') + + assert result == 0 + + out = json.loads(capsys.readouterr().out) + assert out['name'] == 'Nom' + + QUERY_PARAMS = { 'search': ('--query', 'somewhere'), 'reverse': ('--lat', '20', '--lon', '30'), @@ -105,7 +163,7 @@ QUERY_PARAMS = { 'details': ('--node', '324') } -@pytest.mark.parametrize("endpoint", (('search', 'reverse', 'lookup'))) +@pytest.mark.parametrize("endpoint", (('search', 'lookup'))) class TestCliApiCommonParameters: @pytest.fixture(autouse=True) -- 2.39.5