Better parents/scope support, more tests

python-package/lets_plot/geo_data/gis/geocoding_service.py

@@ -13,9 +13,6 @@
 
 class GeocodingService:
  def do_request(self, request: Request) -> Response:
- return self._execute(request)
-
- def _execute(self, request: Request) -> Response:
  if not has_global_value(GEOCODING_PROVIDER_URL):
  raise ValueError('Geocoding server url is not defined')
 

python-package/lets_plot/geo_data/new_api.py

@@ -30,8 +30,6 @@ def _ensure_is_parent_list(obj):
  if obj is None:
  return None
 
- if isinstance(obj, str):
- return [obj]
  if isinstance(obj, Regions):
  return obj.as_list()
 
@@ -40,7 +38,7 @@ def _ensure_is_parent_list(obj):
 
  return [obj]
 
-def _make_parents(values) -> List[Optional[MapRegion]]:
+def _make_parents(values: parent_types) -> List[Optional[MapRegion]]:
  values = _ensure_is_parent_list(values)
 
  if values is None:
@@ -76,15 +74,15 @@ def highlights(self, v: bool) -> 'RegionsBuilder2':
  self._highlights = v
  return self
 
- def countries(self, countries) -> 'RegionsBuilder2':
+ def countries(self, countries: parent_types) -> 'RegionsBuilder2':
  self._countries = _make_parents(countries)
  return self
 
- def states(self, states) -> 'RegionsBuilder2':
+ def states(self, states: parent_types) -> 'RegionsBuilder2':
  self._states = _make_parents(states)
  return self
 
- def counties(self, counties) -> 'RegionsBuilder2':
+ def counties(self, counties: parent_types) -> 'RegionsBuilder2':
  self._counties = _make_parents(counties)
  return self
 
@@ -165,15 +163,19 @@ def _validate_parents_size(parents: List, parents_level: str):
 
  return request
 
+ def _build_regions(self, response: Response, queries: List[RegionQuery]) -> Regions:
+ if not isinstance(response, SuccessResponse):
+ _raise_exception(response)
+
+ return Regions(response.level, response.answers, queries, self._highlights)
+
+
  def build(self) -> Regions:
  request: GeocodingRequest = self._build_request()
 
  response: Response = GeocodingService().do_request(request)
 
- if not isinstance(response, SuccessResponse):
- _raise_exception(response)
-
- return Regions(response.level, response.answers, request.region_queries, self._highlights)
+ return self._build_regions(response, request.region_queries)
 
 
  def __eq__(self, o):

python-package/lets_plot/geo_data/regions.py

@@ -335,7 +335,7 @@ def _get_features(self, feature_id: str) -> List[GeocodedFeature]:
 
 request_types = Optional[Union[str, List[str], Series]]
 scope_types = Optional[Union[str, List[str], Regions, List[Regions]]]
-parent_types = Optional[Union[str, Regions]]
+parent_types = Optional[Union[str, Regions, MapRegion, List]] # list of same types
 
 
 def _raise_exception(response: Response):

python-package/test/geo_data/request_assertion.py

@@ -1,39 +1,55 @@
 # Copyright (c) 2020. JetBrains s.r.o.
 # Use of this source code is governed by the MIT license that can be found in the LICENSE file.
-from typing import TypeVar, Generic, Optional, List
+from typing import TypeVar, Generic, Optional, List, Union
 
-from lets_plot.geo_data.gis.request import Request, GeocodingRequest, RegionQuery, MapRegion, AmbiguityResolver, PayloadKind
+from lets_plot.geo_data.regions import _ensure_is_list
+from lets_plot.geo_data.gis.request import Request, GeocodingRequest, RegionQuery, MapRegion, AmbiguityResolver, \
+ PayloadKind, MapRegionKind
 
 T = TypeVar('T')
 
+
 class ValueMatcher(Generic[T]):
  def check(self, value):
  raise ValueError('abstract')
 
+
 class any(ValueMatcher[T]):
  def check(self, value):
  return
 
+
 class eq(ValueMatcher[T]):
  def __init__(self, v):
  self.expected = v
 
  def check(self, value):
  assert self.expected == value, '{} != {}'.format(self.expected, value)
 
-class eq_map_region_with_name(eq):
+
+class eq_map_region_with_name(eq[MapRegion]):
  def __init__(self, name: str):
  self.expected = MapRegion.with_name(name)
 
-class eq_map_region_with_id(eq):
- def __init__(self, ids: List[str]):
+
+class eq_map_region_with_id(ValueMatcher[MapRegion]):
+ """
+ Checks only id
+ """
+ def __init__(self, ids: Union[str, List[str]]):
+ ids = _ensure_is_list(ids)
  self.expected = MapRegion.scope(ids)
 
+ def check(self, value):
+ assert value.kind == MapRegionKind.id or value.kind == MapRegionKind.place
+ assert self.expected.values == value.values
+
 
 class empty(ValueMatcher[T]):
  def check(self, value):
  assert value is None, '{} is not None'.format(value)
 
+
 class item_exists(ValueMatcher[T]):
  def __init__(self, value):
  self._expected = value
@@ -55,6 +71,7 @@ class ScopeMatcher:
  Scope with ids should have length exactly 1.
 
  '''
+
  def __init__(self):
  self._names: Optional[List[str]] = None
  self._ids: Optional[List[str]] = None
@@ -128,6 +145,7 @@ def check(self, q: RegionQuery):
  self._state.check(q.state)
  self._county.check(q.county)
 
+
 class GeocodingRequestAssertion:
  def __init__(self, request: Request):
  assert isinstance(request, GeocodingRequest)

python-package/test/geo_data/test_new_api.py

@@ -8,7 +8,7 @@
 from lets_plot.geo_data.regions import Regions
 from .geo_data import make_answer
 from .request_assertion import GeocodingRequestAssertion, QueryMatcher, ScopeMatcher, ValueMatcher, eq, empty, \
- eq_map_region_with_name
+ eq_map_region_with_name, eq_map_region_with_id
 
 
 def test_simple():
@@ -30,23 +30,49 @@ def test_no_parents_where_should_override_scope():
  .has_query(0, no_parents(request=eq('foo'), scope=eq_map_region_with_name('bar')))
 
 
+def test_when_regions_in_parent_should_take_region_id():
+ builder = RegionsBuilder2(request='foo') \
+ .states(make_simple_region('bar'))
+
+ assert_that(builder) \
+ .has_query(0, QueryMatcher()
+ .with_name('foo')\
+ .state(eq_map_region_with_id('bar_id'))
+ )
+
+
+def test_parents_can_contain_nulls():
+ builder = RegionsBuilder2(request=['foo', 'bar'])\
+ .states([None, 'baz'])
+
+ assert_that(builder) \
+ .has_query(0, QueryMatcher()
+ .with_name('foo') \
+ .state(empty())
+ ) \
+ .has_query(1, QueryMatcher()
+ .with_name('bar') \
+ .state(eq_map_region_with_name('baz'))
+ )
+
+
 def test_where_with_given_parents_and_duplicated_names():
  # should update scope only for matching name and parents - query with index 1
 
  request = RegionsBuilder2(request=['foo', 'foo']) \
- .counties(['bar', 'baz']) \
- .where(name='foo', county='baz', scope='spam') \
+ .states(['bar', 'baz']) \
+ .where(name='foo', state='baz', scope='spam') \
  ._build_request()
 
  assert_that(request) \
  .has_query(0, QueryMatcher()
  .with_name('foo')
- .county(eq_map_region_with_name('bar'))
+ .state(eq_map_region_with_name('bar'))
  .scope(empty())
  ) \
  .has_query(1, QueryMatcher()
  .with_name('foo')
- .county(eq_map_region_with_name('baz'))
+ .state(eq_map_region_with_name('baz'))
  .scope(eq_map_region_with_name('spam'))
  )
 
@@ -114,7 +140,7 @@ def test_error_where_scope_len_is_invalid():
  )
 
 
-def make_simple_region(requests: Union[str, List[str]], geo_object_ids: Union[str, List[str]] = None) -> Regions:
+def make_simple_region(requests: Union[str, List[str]], geo_object_ids: Union[str, List[str]] = None, level_kind: LevelKind = LevelKind.county) -> Regions:
  requests = requests if isinstance(requests, (list, tuple)) else [requests]
  geo_object_ids = geo_object_ids if geo_object_ids is not None else [request + '_id' for request in requests]
  geo_object_ids = geo_object_ids if isinstance(geo_object_ids, (list, tuple)) else [geo_object_ids]
@@ -125,7 +151,7 @@ def make_simple_region(requests: Union[str, List[str]], geo_object_ids: Union[st
  queries.append(RegionQuery(request=request))
  answers.append(make_answer(request, id, []))
 
- return Regions(LevelKind.county, answers, queries)
+ return Regions(level_kind, answers, queries)
 
 
 def no_parents(request: ValueMatcher[Optional[str]],
@@ -136,7 +162,7 @@ def no_parents(request: ValueMatcher[Optional[str]],
  country=empty(), state=empty(), county=empty())
 
 
-def assert_that(request):
+def assert_that(request: Union[RegionsBuilder2, GeocodingRequest]):
  if isinstance(request, RegionsBuilder2):
  return GeocodingRequestAssertion(request._build_request())
  elif isinstance(request, GeocodingRequest):