From: Sarah Hoffmann Date: Thu, 18 May 2023 20:27:18 +0000 (+0200) Subject: Merge pull request #3063 from lonvia/variable-parameters X-Git-Tag: v4.3.0~76 X-Git-Url: https://git.openstreetmap.org./nominatim.git/commitdiff_plain/d2c56f9f96bce00d9d9309ddf230f825916eccae?hp=9036bf3398fac5778f3d3242b2da58a3b16a3a0d Merge pull request #3063 from lonvia/variable-parameters Rework how search parameters are handed to the Python API --- diff --git a/nominatim/api/__init__.py b/nominatim/api/__init__.py index 9f362379..794cd96c 100644 --- a/nominatim/api/__init__.py +++ b/nominatim/api/__init__.py @@ -23,7 +23,6 @@ from .types import (PlaceID as PlaceID, Point as Point, Bbox as Bbox, GeometryFormat as GeometryFormat, - LookupDetails as LookupDetails, DataLayer as DataLayer) from .results import (SourceTable as SourceTable, AddressLine as AddressLine, diff --git a/nominatim/api/core.py b/nominatim/api/core.py index 3e4b175a..f1a656da 100644 --- a/nominatim/api/core.py +++ b/nominatim/api/core.py @@ -23,7 +23,7 @@ from nominatim.api.connection import SearchConnection from nominatim.api.status import get_status, StatusResult from nominatim.api.lookup import get_detailed_place, get_simple_place from nominatim.api.reverse import ReverseGeocoder -from nominatim.api.types import PlaceRef, LookupDetails, AnyPoint, DataLayer +import nominatim.api.types as ntyp from nominatim.api.results import DetailedResult, ReverseResult, SearchResults @@ -128,32 +128,28 @@ class NominatimAPIAsync: return status - async def details(self, place: PlaceRef, - details: Optional[LookupDetails] = None) -> Optional[DetailedResult]: + async def details(self, place: ntyp.PlaceRef, **params: Any) -> Optional[DetailedResult]: """ Get detailed information about a place in the database. Returns None if there is no entry under the given ID. """ async with self.begin() as conn: - return await get_detailed_place(conn, place, details or LookupDetails()) + return await get_detailed_place(conn, place, + ntyp.LookupDetails.from_kwargs(params)) - async def lookup(self, places: Sequence[PlaceRef], - details: Optional[LookupDetails] = None) -> SearchResults: + async def lookup(self, places: Sequence[ntyp.PlaceRef], **params: Any) -> SearchResults: """ Get simple information about a list of places. Returns a list of place information for all IDs that were found. """ - if details is None: - details = LookupDetails() + details = ntyp.LookupDetails.from_kwargs(params) async with self.begin() as conn: return SearchResults(filter(None, [await get_simple_place(conn, p, details) for p in places])) - async def reverse(self, coord: AnyPoint, max_rank: Optional[int] = None, - layer: Optional[DataLayer] = None, - details: Optional[LookupDetails] = None) -> Optional[ReverseResult]: + async def reverse(self, coord: ntyp.AnyPoint, **params: Any) -> Optional[ReverseResult]: """ Find a place by its coordinates. Also known as reverse geocoding. Returns the closest result that can be found or None if @@ -164,14 +160,8 @@ class NominatimAPIAsync: # There are no results to be expected outside valid coordinates. return None - if layer is None: - layer = DataLayer.ADDRESS | DataLayer.POI - - max_rank = max(0, min(max_rank or 30, 30)) - async with self.begin() as conn: - geocoder = ReverseGeocoder(conn, max_rank, layer, - details or LookupDetails()) + geocoder = ReverseGeocoder(conn, ntyp.ReverseDetails.from_kwargs(params)) return await geocoder.lookup(coord) @@ -206,29 +196,24 @@ class NominatimAPI: return self._loop.run_until_complete(self._async_api.status()) - def details(self, place: PlaceRef, - details: Optional[LookupDetails] = None) -> Optional[DetailedResult]: + def details(self, place: ntyp.PlaceRef, **params: Any) -> Optional[DetailedResult]: """ Get detailed information about a place in the database. """ - return self._loop.run_until_complete(self._async_api.details(place, details)) + return self._loop.run_until_complete(self._async_api.details(place, **params)) - def lookup(self, places: Sequence[PlaceRef], - details: Optional[LookupDetails] = None) -> SearchResults: + def lookup(self, places: Sequence[ntyp.PlaceRef], **params: Any) -> SearchResults: """ Get simple information about a list of places. Returns a list of place information for all IDs that were found. """ - return self._loop.run_until_complete(self._async_api.lookup(places, details)) + return self._loop.run_until_complete(self._async_api.lookup(places, **params)) - def reverse(self, coord: AnyPoint, max_rank: Optional[int] = None, - layer: Optional[DataLayer] = None, - details: Optional[LookupDetails] = None) -> Optional[ReverseResult]: + def reverse(self, coord: ntyp.AnyPoint, **params: Any) -> Optional[ReverseResult]: """ Find a place by its coordinates. Also known as reverse geocoding. Returns the closest result that can be found or None if no place matches the given criteria. """ - return self._loop.run_until_complete( - self._async_api.reverse(coord, max_rank, layer, details)) + return self._loop.run_until_complete(self._async_api.reverse(coord, **params)) diff --git a/nominatim/api/reverse.py b/nominatim/api/reverse.py index 42fe8f36..d6976c06 100644 --- a/nominatim/api/reverse.py +++ b/nominatim/api/reverse.py @@ -16,7 +16,7 @@ from nominatim.typing import SaColumn, SaSelect, SaFromClause, SaLabel, SaRow from nominatim.api.connection import SearchConnection import nominatim.api.results as nres from nominatim.api.logging import log -from nominatim.api.types import AnyPoint, DataLayer, LookupDetails, GeometryFormat, Bbox +from nominatim.api.types import AnyPoint, DataLayer, ReverseDetails, GeometryFormat, Bbox # In SQLAlchemy expression which compare with NULL need to be expressed with # the equal sign. @@ -87,23 +87,34 @@ class ReverseGeocoder: coordinate. """ - def __init__(self, conn: SearchConnection, max_rank: int, layer: DataLayer, - details: LookupDetails) -> None: + def __init__(self, conn: SearchConnection, params: ReverseDetails) -> None: self.conn = conn - self.max_rank = max_rank - self.layer = layer - self.details = details + self.params = params + + + @property + def max_rank(self) -> int: + """ Return the maximum configured rank. + """ + return self.params.max_rank + + + def has_geometries(self) -> bool: + """ Check if any geometries are requested. + """ + return bool(self.params.geometry_output) + def layer_enabled(self, *layer: DataLayer) -> bool: """ Return true when any of the given layer types are requested. """ - return any(self.layer & l for l in layer) + return any(self.params.layers & l for l in layer) def layer_disabled(self, *layer: DataLayer) -> bool: """ Return true when none of the given layer types is requested. """ - return not any(self.layer & l for l in layer) + return not any(self.params.layers & l for l in layer) def has_feature_layers(self) -> bool: @@ -112,21 +123,21 @@ class ReverseGeocoder: return self.layer_enabled(DataLayer.RAILWAY, DataLayer.MANMADE, DataLayer.NATURAL) def _add_geometry_columns(self, sql: SaSelect, col: SaColumn) -> SaSelect: - if not self.details.geometry_output: + if not self.has_geometries(): return sql out = [] - if self.details.geometry_simplification > 0.0: - col = col.ST_SimplifyPreserveTopology(self.details.geometry_simplification) + if self.params.geometry_simplification > 0.0: + col = col.ST_SimplifyPreserveTopology(self.params.geometry_simplification) - if self.details.geometry_output & GeometryFormat.GEOJSON: + if self.params.geometry_output & GeometryFormat.GEOJSON: out.append(col.ST_AsGeoJSON().label('geometry_geojson')) - if self.details.geometry_output & GeometryFormat.TEXT: + if self.params.geometry_output & GeometryFormat.TEXT: out.append(col.ST_AsText().label('geometry_text')) - if self.details.geometry_output & GeometryFormat.KML: + if self.params.geometry_output & GeometryFormat.KML: out.append(col.ST_AsKML().label('geometry_kml')) - if self.details.geometry_output & GeometryFormat.SVG: + if self.params.geometry_output & GeometryFormat.SVG: out.append(col.ST_AsSVG().label('geometry_svg')) return sql.add_columns(*out) @@ -224,7 +235,7 @@ class ReverseGeocoder: if parent_place_id is not None: sql = sql.where(t.c.parent_place_id == parent_place_id) - inner = sql.subquery() + inner = sql.subquery('ipol') sql = sa.select(inner.c.place_id, inner.c.osm_id, inner.c.parent_place_id, inner.c.address, @@ -233,9 +244,9 @@ class ReverseGeocoder: inner.c.postcode, inner.c.country_code, inner.c.distance) - if self.details.geometry_output: - sub = sql.subquery() - sql = self._add_geometry_columns(sql, sub.c.centroid) + if self.has_geometries(): + sub = sql.subquery('geom') + sql = self._add_geometry_columns(sa.select(sub), sub.c.centroid) return (await self.conn.execute(sql)).one_or_none() @@ -252,7 +263,7 @@ class ReverseGeocoder: .where(t.c.parent_place_id == parent_place_id)\ .order_by('distance')\ .limit(1)\ - .subquery() + .subquery('tiger') sql = sa.select(inner.c.place_id, inner.c.parent_place_id, @@ -263,9 +274,9 @@ class ReverseGeocoder: inner.c.postcode, inner.c.distance) - if self.details.geometry_output: - sub = sql.subquery() - sql = self._add_geometry_columns(sql, sub.c.centroid) + if self.has_geometries(): + sub = sql.subquery('geom') + sql = self._add_geometry_columns(sa.select(sub), sub.c.centroid) return (await self.conn.execute(sql)).one_or_none() @@ -345,7 +356,7 @@ class ReverseGeocoder: .where(t.c.type != 'postcode')\ .order_by(sa.desc(t.c.rank_search))\ .limit(50)\ - .subquery() + .subquery('area') sql = _select_from_placex(inner)\ .where(inner.c.geometry.ST_Contains(wkt))\ @@ -374,7 +385,7 @@ class ReverseGeocoder: .intersects(wkt))\ .order_by(sa.desc(t.c.rank_search))\ .limit(50)\ - .subquery() + .subquery('places') touter = self.conn.t.placex.alias('outer') sql = _select_from_placex(inner)\ @@ -514,9 +525,7 @@ class ReverseGeocoder: """ Look up a single coordinate. Returns the place information, if a place was found near the coordinates or None otherwise. """ - log().function('reverse_lookup', - coord=coord, max_rank=self.max_rank, - layer=self.layer, details=self.details) + log().function('reverse_lookup', coord=coord, params=self.params) wkt = WKTElement(f'POINT({coord[0]} {coord[1]})', srid=4326) @@ -539,6 +548,6 @@ class ReverseGeocoder: result.distance = row.distance if hasattr(row, 'bbox'): result.bbox = Bbox.from_wkb(row.bbox.data) - await nres.add_result_details(self.conn, result, self.details) + await nres.add_result_details(self.conn, result, self.params) return result diff --git a/nominatim/api/types.py b/nominatim/api/types.py index e262935a..0e4340fe 100644 --- a/nominatim/api/types.py +++ b/nominatim/api/types.py @@ -7,11 +7,13 @@ """ Complex datatypes used by the Nominatim API. """ -from typing import Optional, Union, Tuple, NamedTuple +from typing import Optional, Union, Tuple, NamedTuple, TypeVar, Type, Dict, Any import dataclasses import enum from struct import unpack +from nominatim.errors import UsageError + @dataclasses.dataclass class PlaceID: """ Reference an object by Nominatim's internal ID. @@ -164,10 +166,22 @@ class GeometryFormat(enum.Flag): TEXT = enum.auto() +class DataLayer(enum.Flag): + """ Layer types that can be selected for reverse and forward search. + """ + POI = enum.auto() + ADDRESS = enum.auto() + RAILWAY = enum.auto() + MANMADE = enum.auto() + NATURAL = enum.auto() + + +TParam = TypeVar('TParam', bound='LookupDetails') # pylint: disable=invalid-name + @dataclasses.dataclass class LookupDetails: """ Collection of parameters that define the amount of details - returned with a search result. + returned with a lookup or details result. """ geometry_output: GeometryFormat = GeometryFormat.NONE """ Add the full geometry of the place to the result. Multiple @@ -194,12 +208,39 @@ class LookupDetails: more the geometry gets simplified. """ + @classmethod + def from_kwargs(cls: Type[TParam], kwargs: Dict[str, Any]) -> TParam: + """ Load the data fields of the class from a dictionary. + Unknown entries in the dictionary are ignored, missing ones + get the default setting. -class DataLayer(enum.Flag): - """ Layer types that can be selected for reverse and forward search. + The function supports type checking and throws a UsageError + when the value does not fit. + """ + def _check_field(v: Any, field: 'dataclasses.Field[Any]') -> Any: + if v is None: + return field.default_factory() \ + if field.default_factory != dataclasses.MISSING \ + else field.default + if field.metadata and 'transform' in field.metadata: + return field.metadata['transform'](v) + if not isinstance(v, field.type): + raise UsageError(f"Parameter '{field.name}' needs to be of {field.type!s}.") + return v + + return cls(**{f.name: _check_field(kwargs[f.name], f) + for f in dataclasses.fields(cls) if f.name in kwargs}) + + +@dataclasses.dataclass +class ReverseDetails(LookupDetails): + """ Collection of parameters for the reverse call. + """ + max_rank: int = dataclasses.field(default=30, + metadata={'transform': lambda v: max(0, min(v, 30))} + ) + """ Highest address rank to return. + """ + layers: DataLayer = DataLayer.ADDRESS | DataLayer.POI + """ Filter which kind of data to include. """ - POI = enum.auto() - ADDRESS = enum.auto() - RAILWAY = enum.auto() - MANMADE = enum.auto() - NATURAL = enum.auto() diff --git a/nominatim/api/v1/helpers.py b/nominatim/api/v1/helpers.py new file mode 100644 index 00000000..e16a22a3 --- /dev/null +++ b/nominatim/api/v1/helpers.py @@ -0,0 +1,31 @@ +# SPDX-License-Identifier: GPL-3.0-or-later +# +# This file is part of Nominatim. (https://nominatim.org) +# +# Copyright (C) 2023 by the Nominatim developer community. +# For a full list of authors see the git log. +""" +Helper function for parsing parameters and and outputting data +specifically for the v1 version of the API. +""" + +REVERSE_MAX_RANKS = [2, 2, 2, # 0-2 Continent/Sea + 4, 4, # 3-4 Country + 8, # 5 State + 10, 10, # 6-7 Region + 12, 12, # 8-9 County + 16, 17, # 10-11 City + 18, # 12 Town + 19, # 13 Village/Suburb + 22, # 14 Hamlet/Neighbourhood + 25, # 15 Localities + 26, # 16 Major Streets + 27, # 17 Minor Streets + 30 # 18 Building + ] + + +def zoom_to_rank(zoom: int) -> int: + """ Convert a zoom parameter into a rank according to the v1 API spec. + """ + return REVERSE_MAX_RANKS[max(0, min(18, zoom))] diff --git a/nominatim/api/v1/server_glue.py b/nominatim/api/v1/server_glue.py index 68cf58c2..ccf8f7d1 100644 --- a/nominatim/api/v1/server_glue.py +++ b/nominatim/api/v1/server_glue.py @@ -8,7 +8,7 @@ Generic part of the server implementation of the v1 API. Combine with the scaffolding provided for the various Python ASGI frameworks. """ -from typing import Optional, Any, Type, Callable, NoReturn, cast +from typing import Optional, Any, Type, Callable, NoReturn, Dict, cast from functools import reduce import abc import math @@ -17,6 +17,7 @@ from nominatim.config import Configuration import nominatim.api as napi import nominatim.api.logging as loglib from nominatim.api.v1.format import dispatch as formatting +from nominatim.api.v1 import helpers CONTENT_TYPE = { 'text': 'text/plain; charset=utf-8', @@ -226,31 +227,32 @@ class ASGIAdaptor(abc.ABC): return fmt - def parse_geometry_details(self, fmt: str) -> napi.LookupDetails: + def parse_geometry_details(self, fmt: str) -> Dict[str, Any]: """ Create details strucutre from the supplied geometry parameters. """ - details = napi.LookupDetails(address_details=True, - geometry_simplification= - self.get_float('polygon_threshold', 0.0)) numgeoms = 0 + output = napi.GeometryFormat.NONE if self.get_bool('polygon_geojson', False): - details.geometry_output |= napi.GeometryFormat.GEOJSON + output |= napi.GeometryFormat.GEOJSON numgeoms += 1 if fmt not in ('geojson', 'geocodejson'): if self.get_bool('polygon_text', False): - details.geometry_output |= napi.GeometryFormat.TEXT + output |= napi.GeometryFormat.TEXT numgeoms += 1 if self.get_bool('polygon_kml', False): - details.geometry_output |= napi.GeometryFormat.KML + output |= napi.GeometryFormat.KML numgeoms += 1 if self.get_bool('polygon_svg', False): - details.geometry_output |= napi.GeometryFormat.SVG + output |= napi.GeometryFormat.SVG numgeoms += 1 if numgeoms > self.config().get_int('POLYGON_OUTPUT_MAX_TYPES'): self.raise_error('Too many polgyon output options selected.') - return details + return {'address_details': True, + 'geometry_simplification': self.get_float('polygon_threshold', 0.0), + 'geometry_output': output + } async def status_endpoint(api: napi.NominatimAPIAsync, params: ASGIAdaptor) -> Any: @@ -285,17 +287,17 @@ async def details_endpoint(api: napi.NominatimAPIAsync, params: ASGIAdaptor) -> debug = params.setup_debugging() - details = napi.LookupDetails(address_details=params.get_bool('addressdetails', False), - linked_places=params.get_bool('linkedplaces', False), - parented_places=params.get_bool('hierarchy', False), - keywords=params.get_bool('keywords', False)) - - if params.get_bool('polygon_geojson', False): - details.geometry_output = napi.GeometryFormat.GEOJSON - locales = napi.Locales.from_accept_languages(params.get_accepted_languages()) - result = await api.details(place, details) + result = await api.details(place, + address_details=params.get_bool('addressdetails', False), + linked_places=params.get_bool('linkedplaces', False), + parented_places=params.get_bool('hierarchy', False), + keywords=params.get_bool('keywords', False), + geometry_output = napi.GeometryFormat.GEOJSON + if params.get_bool('polygon_geojson', False) + else napi.GeometryFormat.NONE + ) if debug: return params.build_response(loglib.get_and_disable()) @@ -318,15 +320,12 @@ async def reverse_endpoint(api: napi.NominatimAPIAsync, params: ASGIAdaptor) -> debug = params.setup_debugging() coord = napi.Point(params.get_float('lon'), params.get_float('lat')) locales = napi.Locales.from_accept_languages(params.get_accepted_languages()) - details = params.parse_geometry_details(fmt) - - zoom = max(0, min(18, params.get_int('zoom', 18))) + details = params.parse_geometry_details(fmt) + details['max_rank'] = helpers.zoom_to_rank(params.get_int('zoom', 18)) + details['layers'] = params.get_layers() - result = await api.reverse(coord, REVERSE_MAX_RANKS[zoom], - params.get_layers() or - napi.DataLayer.ADDRESS | napi.DataLayer.POI, - details) + result = await api.reverse(coord, **details) if debug: return params.build_response(loglib.get_and_disable()) @@ -357,7 +356,7 @@ async def lookup_endpoint(api: napi.NominatimAPIAsync, params: ASGIAdaptor) -> A places.append(napi.OsmID(oid[0], int(oid[1:]))) if places: - results = await api.lookup(places, details) + results = await api.lookup(places, **details) else: results = napi.SearchResults() @@ -375,22 +374,6 @@ async def lookup_endpoint(api: napi.NominatimAPIAsync, params: ASGIAdaptor) -> A EndpointFunc = Callable[[napi.NominatimAPIAsync, ASGIAdaptor], Any] -REVERSE_MAX_RANKS = [2, 2, 2, # 0-2 Continent/Sea - 4, 4, # 3-4 Country - 8, # 5 State - 10, 10, # 6-7 Region - 12, 12, # 8-9 County - 16, 17, # 10-11 City - 18, # 12 Town - 19, # 13 Village/Suburb - 22, # 14 Hamlet/Neighbourhood - 25, # 15 Localities - 26, # 16 Major Streets - 27, # 17 Minor Streets - 30 # 18 Building - ] - - ROUTES = [ ('status', status_endpoint), ('details', details_endpoint), diff --git a/nominatim/clicmd/api.py b/nominatim/clicmd/api.py index 58edbea4..82ebb2c1 100644 --- a/nominatim/clicmd/api.py +++ b/nominatim/clicmd/api.py @@ -18,7 +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 +from nominatim.api.v1.helpers import zoom_to_rank # Do not repeat documentation of subcommand classes. # pylint: disable=C0111 @@ -163,14 +163,12 @@ class APIReverse: def run(self, args: NominatimArgs) -> int: api = napi.NominatimAPI(args.project_dir) - 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) + max_rank=zoom_to_rank(args.zoom or 18), + layers=args.get_layers(napi.DataLayer.ADDRESS | napi.DataLayer.POI), + address_details=True, # needed for display name + geometry_output=args.get_geometry_output(), + geometry_simplification=args.polygon_threshold) if result: output = api_output.format_result( @@ -216,13 +214,12 @@ class APILookup: def run(self, args: NominatimArgs) -> int: api = napi.NominatimAPI(args.project_dir) - details = napi.LookupDetails(address_details=True, # needed for display name - geometry_output=args.get_geometry_output(), - geometry_simplification=args.polygon_threshold or 0.0) - places = [napi.OsmID(o[0], int(o[1:])) for o in args.ids] - results = api.lookup(places, details) + results = api.lookup(places, + address_details=True, # needed for display name + geometry_output=args.get_geometry_output(), + geometry_simplification=args.polygon_threshold or 0.0) output = api_output.format_result( results, @@ -297,14 +294,15 @@ class APIDetails: api = napi.NominatimAPI(args.project_dir) - details = napi.LookupDetails(address_details=args.addressdetails, - linked_places=args.linkedplaces, - parented_places=args.hierarchy, - keywords=args.keywords) - if args.polygon_geojson: - details.geometry_output = napi.GeometryFormat.GEOJSON + result = api.details(place, + address_details=args.addressdetails, + linked_places=args.linkedplaces, + parented_places=args.hierarchy, + keywords=args.keywords, + geometry_output=napi.GeometryFormat.GEOJSON + if args.polygon_geojson + else napi.GeometryFormat.NONE) - result = api.details(place, details) if result: output = api_output.format_result( diff --git a/test/python/api/test_api_details.py b/test/python/api/test_api_details.py index 625c4e7a..101dfd13 100644 --- a/test/python/api/test_api_details.py +++ b/test/python/api/test_api_details.py @@ -31,7 +31,7 @@ def test_lookup_in_placex(apiobj, idobj): indexed_date=import_date, geometry='LINESTRING(23 34, 23.1 34, 23.1 34.1, 23 34)') - result = apiobj.api.details(idobj, napi.LookupDetails()) + result = apiobj.api.details(idobj) assert result is not None @@ -79,7 +79,7 @@ def test_lookup_in_placex_minimal_info(apiobj): indexed_date=import_date, geometry='LINESTRING(23 34, 23.1 34, 23.1 34.1, 23 34)') - result = apiobj.api.details(napi.PlaceID(332), napi.LookupDetails()) + result = apiobj.api.details(napi.PlaceID(332)) assert result is not None @@ -121,8 +121,7 @@ def test_lookup_in_placex_with_geometry(apiobj): apiobj.add_placex(place_id=332, geometry='LINESTRING(23 34, 23.1 34)') - result = apiobj.api.details(napi.PlaceID(332), - napi.LookupDetails(geometry_output=napi.GeometryFormat.GEOJSON)) + result = apiobj.api.details(napi.PlaceID(332), geometry_output=napi.GeometryFormat.GEOJSON) assert result.geometry == {'geojson': '{"type":"LineString","coordinates":[[23,34],[23.1,34]]}'} @@ -144,8 +143,7 @@ def test_lookup_placex_with_address_details(apiobj): country_code='pl', rank_search=17, rank_address=16) - result = apiobj.api.details(napi.PlaceID(332), - napi.LookupDetails(address_details=True)) + result = apiobj.api.details(napi.PlaceID(332), address_details=True) assert result.address_rows == [ napi.AddressLine(place_id=332, osm_object=('W', 4), @@ -177,8 +175,7 @@ def test_lookup_place_with_linked_places_none_existing(apiobj): country_code='pl', linked_place_id=45, rank_search=27, rank_address=26) - result = apiobj.api.details(napi.PlaceID(332), - napi.LookupDetails(linked_places=True)) + result = apiobj.api.details(napi.PlaceID(332), linked_places=True) assert result.linked_rows == [] @@ -197,8 +194,7 @@ def test_lookup_place_with_linked_places_existing(apiobj): country_code='pl', linked_place_id=332, rank_search=27, rank_address=26) - result = apiobj.api.details(napi.PlaceID(332), - napi.LookupDetails(linked_places=True)) + result = apiobj.api.details(napi.PlaceID(332), linked_places=True) assert result.linked_rows == [ napi.AddressLine(place_id=1001, osm_object=('W', 5), @@ -220,8 +216,7 @@ def test_lookup_place_with_parented_places_not_existing(apiobj): country_code='pl', parent_place_id=45, rank_search=27, rank_address=26) - result = apiobj.api.details(napi.PlaceID(332), - napi.LookupDetails(parented_places=True)) + result = apiobj.api.details(napi.PlaceID(332), parented_places=True) assert result.parented_rows == [] @@ -240,8 +235,7 @@ def test_lookup_place_with_parented_places_existing(apiobj): country_code='pl', parent_place_id=332, rank_search=27, rank_address=26) - result = apiobj.api.details(napi.PlaceID(332), - napi.LookupDetails(parented_places=True)) + result = apiobj.api.details(napi.PlaceID(332), parented_places=True) assert result.parented_rows == [ napi.AddressLine(place_id=1001, osm_object=('N', 5), @@ -263,7 +257,7 @@ def test_lookup_in_osmline(apiobj, idobj): indexed_date=import_date, geometry='LINESTRING(23 34, 23 35)') - result = apiobj.api.details(idobj, napi.LookupDetails()) + result = apiobj.api.details(idobj) assert result is not None @@ -310,13 +304,13 @@ def test_lookup_in_osmline_split_interpolation(apiobj): startnumber=11, endnumber=20, step=1) for i in range(1, 6): - result = apiobj.api.details(napi.OsmID('W', 9, str(i)), napi.LookupDetails()) + result = apiobj.api.details(napi.OsmID('W', 9, str(i))) assert result.place_id == 1000 for i in range(7, 11): - result = apiobj.api.details(napi.OsmID('W', 9, str(i)), napi.LookupDetails()) + result = apiobj.api.details(napi.OsmID('W', 9, str(i))) assert result.place_id == 1001 for i in range(12, 22): - result = apiobj.api.details(napi.OsmID('W', 9, str(i)), napi.LookupDetails()) + result = apiobj.api.details(napi.OsmID('W', 9, str(i))) assert result.place_id == 1002 @@ -340,8 +334,7 @@ def test_lookup_osmline_with_address_details(apiobj): country_code='pl', rank_search=17, rank_address=16) - result = apiobj.api.details(napi.PlaceID(9000), - napi.LookupDetails(address_details=True)) + result = apiobj.api.details(napi.PlaceID(9000), address_details=True) assert result.address_rows == [ napi.AddressLine(place_id=None, osm_object=None, @@ -383,7 +376,7 @@ def test_lookup_in_tiger(apiobj): osm_type='W', osm_id=6601223, geometry='LINESTRING(23 34, 23 35)') - result = apiobj.api.details(napi.PlaceID(4924), napi.LookupDetails()) + result = apiobj.api.details(napi.PlaceID(4924)) assert result is not None @@ -441,8 +434,7 @@ def test_lookup_tiger_with_address_details(apiobj): country_code='us', rank_search=17, rank_address=16) - result = apiobj.api.details(napi.PlaceID(9000), - napi.LookupDetails(address_details=True)) + result = apiobj.api.details(napi.PlaceID(9000), address_details=True) assert result.address_rows == [ napi.AddressLine(place_id=None, osm_object=None, @@ -483,7 +475,7 @@ def test_lookup_in_postcode(apiobj): indexed_date=import_date, geometry='POINT(-9.45 5.6)') - result = apiobj.api.details(napi.PlaceID(554), napi.LookupDetails()) + result = apiobj.api.details(napi.PlaceID(554)) assert result is not None @@ -537,8 +529,7 @@ def test_lookup_postcode_with_address_details(apiobj): country_code='gb', rank_search=17, rank_address=16) - result = apiobj.api.details(napi.PlaceID(9000), - napi.LookupDetails(address_details=True)) + result = apiobj.api.details(napi.PlaceID(9000), address_details=True) assert result.address_rows == [ napi.AddressLine(place_id=332, osm_object=('N', 3333), @@ -570,7 +561,7 @@ def test_lookup_missing_object(apiobj, objid): apiobj.add_placex(place_id=1, osm_type='N', osm_id=55, class_='place', type='suburb') - assert apiobj.api.details(objid, napi.LookupDetails()) is None + assert apiobj.api.details(objid) is None @pytest.mark.parametrize('gtype', (napi.GeometryFormat.KML, @@ -580,5 +571,4 @@ def test_lookup_unsupported_geometry(apiobj, gtype): apiobj.add_placex(place_id=332) with pytest.raises(ValueError): - apiobj.api.details(napi.PlaceID(332), - napi.LookupDetails(geometry_output=gtype)) + apiobj.api.details(napi.PlaceID(332), geometry_output=gtype) diff --git a/test/python/api/test_api_lookup.py b/test/python/api/test_api_lookup.py index 6aafa29e..619bc747 100644 --- a/test/python/api/test_api_lookup.py +++ b/test/python/api/test_api_lookup.py @@ -95,7 +95,7 @@ def test_lookup_multiple_places(apiobj): result = apiobj.api.lookup((napi.OsmID('W', 1), napi.OsmID('W', 4), - napi.OsmID('W', 9928)), napi.LookupDetails()) + napi.OsmID('W', 9928))) assert len(result) == 2 diff --git a/test/python/api/test_api_reverse.py b/test/python/api/test_api_reverse.py index d1d47f84..3296e98f 100644 --- a/test/python/api/test_api_reverse.py +++ b/test/python/api/test_api_reverse.py @@ -84,7 +84,7 @@ def test_reverse_rank_30_layers(apiobj, y, layer, place_id): rank_search=30, centroid=(1.3, 0.70005)) - assert apiobj.api.reverse((1.3, y), layer=layer).place_id == place_id + assert apiobj.api.reverse((1.3, y), layers=layer).place_id == place_id def test_reverse_poi_layer_with_no_pois(apiobj): @@ -95,7 +95,7 @@ def test_reverse_poi_layer_with_no_pois(apiobj): centroid=(1.3, 0.70001)) assert apiobj.api.reverse((1.3, 0.70001), max_rank=29, - layer=napi.DataLayer.POI) is None + layers=napi.DataLayer.POI) is None def test_reverse_housenumber_on_street(apiobj): @@ -245,7 +245,7 @@ def test_reverse_larger_area_layers(apiobj, layer, place_id): rank_search=16, centroid=(1.3, 0.70005)) - assert apiobj.api.reverse((1.3, 0.7), layer=layer).place_id == place_id + assert apiobj.api.reverse((1.3, 0.7), layers=layer).place_id == place_id def test_reverse_country_lookup_no_objects(apiobj): @@ -296,10 +296,8 @@ def test_reverse_geometry_output_placex(apiobj, gtype): country_code='xx', centroid=(0.5, 0.5)) - details = napi.LookupDetails(geometry_output=gtype) - - assert apiobj.api.reverse((59.3, 80.70001), details=details).place_id == 1001 - assert apiobj.api.reverse((0.5, 0.5), details=details).place_id == 1003 + assert apiobj.api.reverse((59.3, 80.70001), geometry_output=gtype).place_id == 1001 + assert apiobj.api.reverse((0.5, 0.5), geometry_output=gtype).place_id == 1003 def test_reverse_simplified_geometry(apiobj): @@ -309,9 +307,9 @@ def test_reverse_simplified_geometry(apiobj): rank_search=30, centroid=(59.3, 80.70001)) - details = napi.LookupDetails(geometry_output=napi.GeometryFormat.GEOJSON, - geometry_simplification=0.1) - assert apiobj.api.reverse((59.3, 80.70001), details=details).place_id == 1001 + details = dict(geometry_output=napi.GeometryFormat.GEOJSON, + geometry_simplification=0.1) + assert apiobj.api.reverse((59.3, 80.70001), **details).place_id == 1001 def test_reverse_interpolation_geometry(apiobj): @@ -321,8 +319,7 @@ def test_reverse_interpolation_geometry(apiobj): centroid=(10.0, 10.00001), geometry='LINESTRING(9.995 10.00001, 10.005 10.00001)') - details = napi.LookupDetails(geometry_output=napi.GeometryFormat.TEXT) - assert apiobj.api.reverse((10.0, 10.0), details=details)\ + assert apiobj.api.reverse((10.0, 10.0), geometry_output=napi.GeometryFormat.TEXT)\ .geometry['text'] == 'POINT(10 10.00001)' @@ -339,8 +336,8 @@ def test_reverse_tiger_geometry(apiobj): centroid=(10.0, 10.00001), geometry='LINESTRING(9.995 10.00001, 10.005 10.00001)') - details = napi.LookupDetails(geometry_output=napi.GeometryFormat.GEOJSON) - output = apiobj.api.reverse((10.0, 10.0), details=details).geometry['geojson'] + output = apiobj.api.reverse((10.0, 10.0), + geometry_output=napi.GeometryFormat.GEOJSON).geometry['geojson'] assert json.loads(output) == {'coordinates': [10, 10.00001], 'type': 'Point'} diff --git a/test/python/api/test_api_types.py b/test/python/api/test_api_types.py new file mode 100644 index 00000000..6a095bcb --- /dev/null +++ b/test/python/api/test_api_types.py @@ -0,0 +1,35 @@ +# SPDX-License-Identifier: GPL-3.0-or-later +# +# This file is part of Nominatim. (https://nominatim.org) +# +# Copyright (C) 2023 by the Nominatim developer community. +# For a full list of authors see the git log. +""" +Tests for loading of parameter dataclasses. +""" +import pytest + +from nominatim.errors import UsageError +import nominatim.api.types as typ + +def test_no_params_defaults(): + params = typ.LookupDetails.from_kwargs({}) + + assert not params.parented_places + assert params.geometry_simplification == 0.0 + + +@pytest.mark.parametrize('k,v', [('geometry_output', 'a'), + ('linked_places', 0), + ('geometry_simplification', 'NaN')]) +def test_bad_format_reverse(k, v): + with pytest.raises(UsageError): + params = typ.ReverseDetails.from_kwargs({k: v}) + + +@pytest.mark.parametrize('rin,rout', [(-23, 0), (0, 0), (1, 1), + (15, 15), (30, 30), (31, 30)]) +def test_rank_params(rin, rout): + params = typ.ReverseDetails.from_kwargs({'max_rank': rin}) + + assert params.max_rank == rout diff --git a/test/python/cli/test_cmd_api.py b/test/python/cli/test_cmd_api.py index e8c447aa..2d7897a3 100644 --- a/test/python/cli/test_cmd_api.py +++ b/test/python/cli/test_cmd_api.py @@ -81,7 +81,7 @@ class TestCliDetailsCall: napi.Point(1.0, -3.0)) monkeypatch.setattr(napi.NominatimAPI, 'details', - lambda *args: result) + lambda *args, **kwargs: result) @pytest.mark.parametrize("params", [('--node', '1'), ('--way', '1'), @@ -106,7 +106,7 @@ class TestCliReverseCall: extratags={'extra':'Extra'}) monkeypatch.setattr(napi.NominatimAPI, 'reverse', - lambda *args: result) + lambda *args, **kwargs: result) def test_reverse_simple(self, cli_call, tmp_path, capsys): @@ -165,7 +165,7 @@ class TestCliLookupCall: extratags={'extra':'Extra'}) monkeypatch.setattr(napi.NominatimAPI, 'lookup', - lambda *args: napi.SearchResults([result])) + lambda *args, **kwargs: napi.SearchResults([result])) def test_lookup_simple(self, cli_call, tmp_path, capsys): result = cli_call('lookup', '--project-dir', str(tmp_path),