Merge pull request #348 from rustprooflabs/fix-nested-place-logic

Fix nested polygon logic when place=false

docker/pgosm_flex.py

@@ -220,7 +220,8 @@ def run_osm2pgsql_standard(input_file, out_path, flex_path, ram, skip_nested,
                   debug=debug)
 
     if not skip_nested:
-        skip_nested = check_layerset_places(flex_path)
+        # Don't expect user to use --skip-nested when place isn't included
+        skip_nested = check_layerset_skip_nested_place(flex_path)
 
     post_processing = run_post_processing(flex_path=flex_path,
                                           skip_nested=skip_nested,
@@ -426,16 +427,48 @@ def run_osm2pgsql(osm2pgsql_command, flex_path, debug):
     logger.info('osm2pgsql completed')
 
 
-def check_layerset_places(flex_path):
+def check_layerset_skip_nested_place(flex_path: str) -> bool:
     """If `place` layer is not included `skip_nested` should be true.
 
     Parameters
     ------------------------
     flex_path : str
+        Path to the .ini file for the defined layerset defining which layers
+        to include.
 
     Returns
     ------------------------
     skip_nested : boolean
+        Return True to disable nested place polygon (post-import processing)
+    """
+    logger = logging.getLogger('pgosm-flex')
+
+    include_place = layerset_include_place(flex_path)
+
+    logger.debug('--------------------------------------------')
+    logger.debug(f'Place layer value: {type(include_place)}')
+    logger.debug(f'Place layer value: {include_place}')
+    logger.debug('--------------------------------------------')
+
+    if include_place:
+        logger.debug('Place layer is defined as true. Not setting skip_nested')
+        return False
+
+    logger.debug('Place layer not included')
+    return True
+
+
+def layerset_include_place(flex_path: str) -> bool:
+    """
+    Parameters
+    ---------------------
+    flex_path : str
+
+    Returns
+    ---------------------
+    place : bool
+        If true, the Place layer will be included and post-processing functions
+        should be called.
     """
     logger = logging.getLogger('pgosm-flex')
     layerset = os.environ.get('PGOSM_LAYERSET')
@@ -448,18 +481,22 @@ def check_layerset_places(flex_path):
     ini_file = os.path.join(layerset_path, f'{layerset}.ini')
     config = configparser.ConfigParser()
     config.read(ini_file)
+
     try:
-        place = config['layerset']['place']
+        # Gets the value from INI
+        # Comes through as str, convert to bool
+        include_place = config['layerset']['place']
+        logger.debug(f'Include place?  {include_place}')
+        if include_place.lower() == 'true':
+            place = True
+        else:
+            place = False
+        logger.debug(f'Include place?  {place}')
     except KeyError:
         logger.debug('Place layer not defined, setting skip_nested')
-        return True
-
-    if place:
-        logger.debug('Place layer is defined as true. Not setting skip_nested')
-        return False
+        place = False
 
-    logger.debug('Place set to false, setting skip_nested')
-    return True
+    return place
 
 
 def run_post_processing(flex_path, skip_nested, import_mode):

docker/tests/layersets/place_false.ini

@@ -0,0 +1,2 @@
+[layerset]
+place=false

docker/tests/layersets/place_missing.ini

@@ -0,0 +1 @@
+[layerset]

docker/tests/layersets/place_true.ini

@@ -0,0 +1,2 @@
+[layerset]
+place=true

docker/tests/test_pgosm_flex.py

@@ -99,3 +99,98 @@ def test_get_export_filename_region_only(self):
         expected = 'north-america-default-2021-12-02.sql'
         self.assertEqual(expected, result)
 
+    def test_layerset_include_place_returns_boolean(self):
+        helpers.unset_env_vars()
+        layerset_path = None
+        helpers.set_env_vars(region='north-america',
+                             subregion=None,
+                             srid=3857,
+                             language=None,
+                             pgosm_date=PGOSM_DATE,
+                             layerset=LAYERSET,
+                             layerset_path=layerset_path,
+                             sp_gist=False,
+                             replication=False)
+
+        paths = pgosm_flex.get_paths()
+        result = pgosm_flex.layerset_include_place(flex_path=paths['flex_path'])
+        expected = bool
+        actual = type(result)
+        self.assertEqual(expected, actual)
+
+    def test_layerset_include_place_returns_True_with_default_layerset(self):
+        helpers.unset_env_vars()
+        layerset_path = None
+        helpers.set_env_vars(region='north-america',
+                             subregion=None,
+                             srid=3857,
+                             language=None,
+                             pgosm_date=PGOSM_DATE,
+                             layerset=LAYERSET,
+                             layerset_path=layerset_path,
+                             sp_gist=False,
+                             replication=False)
+
+        paths = pgosm_flex.get_paths()
+        actual = pgosm_flex.layerset_include_place(flex_path=paths['flex_path'])
+        expected = True
+        self.assertEqual(expected, actual)
+
+    def test_layerset_include_place_returns_false_when_place_false_in_ini(self):
+        helpers.unset_env_vars()
+        layerset_path = '/app/docker/tests/layersets'
+        layerset = 'place_false'
+        helpers.set_env_vars(region='north-america',
+                             subregion=None,
+                             srid=3857,
+                             language=None,
+                             pgosm_date=PGOSM_DATE,
+                             layerset=layerset,
+                             layerset_path=layerset_path,
+                             sp_gist=False,
+                             replication=False)
+
+        paths = pgosm_flex.get_paths()
+        actual = pgosm_flex.layerset_include_place(flex_path=paths['flex_path'])
+        expected = False
+        self.assertEqual(expected, actual)
+
+    def test_layerset_include_place_returns_false_when_place_missing_in_ini(self):
+        helpers.unset_env_vars()
+        layerset_path = '/app/docker/tests/layersets'
+        layerset = 'place_missing'
+        helpers.set_env_vars(region='north-america',
+                             subregion=None,
+                             srid=3857,
+                             language=None,
+                             pgosm_date=PGOSM_DATE,
+                             layerset=layerset,
+                             layerset_path=layerset_path,
+                             sp_gist=False,
+                             replication=False)
+
+        paths = pgosm_flex.get_paths()
+        actual = pgosm_flex.layerset_include_place(flex_path=paths['flex_path'])
+        expected = False
+        self.assertEqual(expected, actual)
+
+    def test_layerset_include_place_returns_true_when_place_true_in_ini(self):
+        helpers.unset_env_vars()
+        layerset_path = '/app/docker/tests/layersets'
+        layerset = 'place_true'
+        helpers.set_env_vars(region='north-america',
+                             subregion=None,
+                             srid=3857,
+                             language=None,
+                             pgosm_date=PGOSM_DATE,
+                             layerset=layerset,
+                             layerset_path=layerset_path,
+                             sp_gist=False,
+                             replication=False)
+
+        paths = pgosm_flex.get_paths()
+        actual = pgosm_flex.layerset_include_place(flex_path=paths['flex_path'])
+        expected = True
+        self.assertEqual(expected, actual)
+
+