[DEV] refactor coding: do/remove checks, adjust exceptions, cleanup c…

…oding (#196)

* move get_ ... _from_input to each class as static method

* move translate_id_no_to_geofabrik to CountryGeofabrik

with unittests

* check if xy coordinate has countries and exit if not

with unittests

* delete unused functions. found with vulture

* unify docs

* remove not needed translations from input country to geofabrik country

- nowadays input is checked against geofabrik countries
- countries are translated in addition when initializing class CountryGeofabrik

docs/FAQ.md

@@ -3,7 +3,7 @@
 - [Usage of wahooMapsCreator](#usage-of-wahoomapscreator)
   - [The tool stops but does not output an error](#the-tool-stops-but-does-not-output-an-error)
   - [I have a Java error. "Java heap space - out of memory"](#i-have-a-java-error-java-heap-space---out-of-memory)
-  - [how can I migrate from v3.x.x to v4.x.x?](#how-can-i-migrate-from-v3xx-to-v4xx)
+  - [How can I migrate from v3.x.x to v4.x.x?](#how-can-i-migrate-from-v3xx-to-v4xx)
 - [Generated maps](#generated-maps)
   - [Where on my Wahoo device do I have to copy the maps?](#where-on-my-wahoo-device-do-i-have-to-copy-the-maps)
   - [How can I restore original maps?](#how-can-i-restore-original-maps)
@@ -24,7 +24,7 @@ If you are on Windows and running out of memory, it can be that you just don't h
 
 You can try to deinstall Java 32 and install the 64 bit version. Other than that you can make changes from this PR https://github.com/treee111/wahooMapsCreator/pull/171/files in the osmosis.bat file on your computer. This file located here: `wahooMapsCreatorData\_download\tooling_win\Osmosis\bin\osmosis.bat`.
 
-### how can I migrate from v3.x.x to v4.x.x?
+### How can I migrate from v3.x.x to v4.x.x?
 You have to remove your existing environment:
 ```
 conda env remove -n gdal-user

docs/QUICKSTART_ANACONDA.md

@@ -10,7 +10,7 @@
     - [homebrew](#homebrew)
     - [OSM-tools](#osm-tools)
 - [wahooMapsCreator](#wahoomapscreator)
-  - [Create Anaconda Environment](#create-anaconda-environment)
+  - [Create Anaconda environment](#create-anaconda-environment)
   - [Install wahooMapsCreator into Anaconda environment](#install-wahoomapscreator-into-anaconda-environment)
     - [Update wahooMapsCreator](#update-wahoomapscreator)
   - [Additional programs for generating contour lines](#additional-programs-for-generating-contour-lines)
@@ -76,7 +76,7 @@ brew install osmosis
 ```
 
 # wahooMapsCreator
-## Create Anaconda Environment
+## Create Anaconda environment
 1. Open terminal (macOS/Linux) or **Anaconda Prompt** (Windows, via Startmenu)
 2. Create a new Anaconda environment with needed packages
 ```

tests/test_constants_geofabrik.py

@@ -4,7 +4,7 @@
 import os
 import unittest
 
-import geojson # pylint: disable=import-error
+import geojson  # pylint: disable=import-error
 
 # import custom python packages
 from wahoomc.downloader import Downloader

tests/test_geofabrik.py

@@ -7,11 +7,11 @@
 from shapely.geometry import shape  # pylint: disable=import-error
 
 # import custom python packages
-from wahoomc.geofabrik import CountryGeofabrik, XYGeofabrik
-from wahoomc.geofabrik import calc_bounding_box_tiles, get_xy_coordinates_from_input
+from wahoomc.geofabrik import CountryGeofabrik, XYCombinationHasNoCountries, XYGeofabrik
+from wahoomc.geofabrik import calc_bounding_box_tiles
 from wahoomc.downloader import Downloader
 from wahoomc import constants
-from wahoomc.geofabrik_json import GeofabrikJson
+from wahoomc.geofabrik_json import CountyIsNoGeofabrikCountry, GeofabrikJson
 
 
 def calc_tiles_via_geofabrik_json(input_argument):
@@ -160,25 +160,61 @@ def test_splitting_of_single_xy_coordinate(self):
         use static json files in the repo to calculate relevant tiles
         """
 
-        xy_tuple = get_xy_coordinates_from_input("133/88")
+        xy_tuple = XYGeofabrik.split_input_to_list("133/88")
         self.assertEqual(xy_tuple, [{"x": 133, "y": 88}])
 
-        xy_tuple = get_xy_coordinates_from_input("11/92")
+        xy_tuple = XYGeofabrik.split_input_to_list("11/92")
         self.assertEqual(xy_tuple, [{"x": 11, "y": 92}])
 
-        xy_tuple = get_xy_coordinates_from_input("138/100")
+        xy_tuple = XYGeofabrik.split_input_to_list("138/100")
         self.assertEqual(xy_tuple, [{"x": 138, "y": 100}])
 
     def test_splitting_of_multiple_xy_coordinate(self):
         """
         use static json files in the repo to calculate relevant tiles
         """
 
-        xy_tuple = get_xy_coordinates_from_input("133/88,138/100")
+        xy_tuple = XYGeofabrik.split_input_to_list("133/88,138/100")
         expected_result = [{"x": 133, "y": 88}, {"x": 138, "y": 100}]
 
         self.assertEqual(xy_tuple, expected_result)
 
+    def test_if_countrygeofabrik_raises_exception(self):
+        """
+        initialize CountryGeofabrik class and let the class check if the country is OK
+        in other words: if the exception raised by translate_id_no_to_geofabrik
+        is transported all up
+        """
+        CountryGeofabrik('germany')
+        CountryGeofabrik('malta')
+        CountryGeofabrik('malta,germany')
+
+        with self.assertRaises(CountyIsNoGeofabrikCountry):
+            CountryGeofabrik('germanyd')
+        with self.assertRaises(CountyIsNoGeofabrikCountry):
+            CountryGeofabrik('xy')
+        with self.assertRaises(CountyIsNoGeofabrikCountry):
+            CountryGeofabrik('maltad,germany')
+        with self.assertRaises(CountyIsNoGeofabrikCountry):
+            CountryGeofabrik('malta,tekke')
+
+    def test_if_xy_geofabrik_raises_exception(self):
+        """
+        initialize XYGeofabrik class + check if xy coordinate has countries
+        """
+        o_geofabrik = XYGeofabrik('133/87')
+        o_geofabrik.get_tiles_of_wanted_map()
+
+        o_geofabrik = XYGeofabrik('100/138')
+        o_geofabrik.get_tiles_of_wanted_map()
+
+        o_geofabrik = XYGeofabrik('138/100')
+        o_geofabrik.get_tiles_of_wanted_map()
+
+        with self.assertRaises(XYCombinationHasNoCountries):
+            o_geofabrik = XYGeofabrik('200/1')
+            o_geofabrik.get_tiles_of_wanted_map()
+
     # def test_get_tile_via_xy_coordinate_error(self):
     #     """
     #     use static json files in the repo to calculate a not-existing tile.

wahoomc/downloader.py

@@ -297,31 +297,25 @@ def check_osm_pbf_file(self):
         log.info('+ Checking for old maps and remove them')
 
         for country in self.border_countries:
-            # get translated country (geofabrik) of country
-            # do not download the same file for different countries
-            # --> e.g. China, Hong Kong and Macao, see Issue #11
-            transl_c = self.o_geofabrik_json.translate_id_no_to_geofabrik(
-                country)
-
             # check for already existing .osm.pbf file
             map_file_path = glob.glob(
-                f'{USER_MAPS_DIR}/{transl_c}-latest.osm.pbf')
+                f'{USER_MAPS_DIR}/{country}-latest.osm.pbf')
             if len(map_file_path) != 1:
                 map_file_path = glob.glob(
-                    f'{USER_MAPS_DIR}/**/{transl_c}-latest.osm.pbf')
+                    f'{USER_MAPS_DIR}/**/{country}-latest.osm.pbf')
 
             # delete .osm.pbf file if out of date
             if len(map_file_path) == 1 and os.path.isfile(map_file_path[0]):
                 if self.should_file_be_downloaded(map_file_path[0]):
                     log.info(
-                        '+ mapfile for %s: deleted. Input: %s.', transl_c, country)
+                        '+ mapfile for %s: deleted.', country)
                     os.remove(map_file_path[0])
                     self.need_to_dl.append('osm_pbf')
                 else:
                     self.border_countries[country] = {
                         'map_file': map_file_path[0]}
                     log.info(
-                        '+ mapfile for %s: up-to-date. Input: %s.', transl_c, country)
+                        '+ mapfile for %s: up-to-date.', country)
 
             # mark country .osm.pbf file for download if there exists no file or it is no file
             map_file_path = self.border_countries[country].get('map_file')
@@ -337,8 +331,7 @@ def download_osm_pbf_file(self):
             try:
                 if item['download'] is True:
                     # build path to downloaded file with translated geofabrik country
-                    map_file_path = build_osm_pbf_filepath(
-                        self.o_geofabrik_json.translate_id_no_to_geofabrik(country))
+                    map_file_path = build_osm_pbf_filepath(country)
                     # fetch the geofabrik download url to countries' OSM file
                     url = self.o_geofabrik_json.get_geofabrik_url(country)
                     download_file(map_file_path, url)

wahoomc/file_directory_functions.py

@@ -108,16 +108,6 @@ def write_json_file_generic(json_file_path, json_content):
         json_file.close()
 
 
-def get_folders_in_folder(folder):
-    """
-    return foldernames of given folder without path as list
-    """
-    onlyfolders = [f for f in os.listdir(
-        folder) if not isfile(join(folder, f))]
-
-    return onlyfolders
-
-
 def get_files_in_folder(folder):
     """
     return filenames of given folder without path as list
@@ -127,21 +117,6 @@ def get_files_in_folder(folder):
     return onlyfiles
 
 
-def get_filenames_of_jsons_in_folder(folder):
-    """
-    return json-file filenames of given folder without path as list
-    """
-    json_files = []
-
-    for file in get_files_in_folder(folder):
-        if file.endswith('.json'):
-            # filename = file.split('.')[0]
-            filename = os.path.splitext(file)[0]
-            json_files.extend([filename])
-
-    return json_files
-
-
 def delete_o5m_pbf_files_in_folder(folder):
     """
     delete .o5m and .pbf files of given folder

wahoomc/geofabrik.py

@@ -12,11 +12,19 @@
 
 # import custom python packages
 from wahoomc.constants import special_regions, block_download
-from wahoomc.geofabrik_json import GeofabrikJson
+from wahoomc.geofabrik_json import CountyIsNoGeofabrikCountry, GeofabrikJson
 
 log = logging.getLogger('main-logger')
 
 
+class XYCombinationHasNoCountries(Exception):
+    """Raised when no tile is found for x/y combination"""
+
+    def __init__(self, xy_coordinate):
+        message = f"Given XY coordinate '{xy_coordinate}' consists of no countries. Please check if your input is valid."
+        super().__init__(message)
+
+
 class InformalGeofabrikInterface:
     """Informal class for Geofabrik processing"""
     wanted_maps = []
@@ -38,8 +46,13 @@ def get_tiles_of_wanted_map(self) -> list:
 
         tiles_of_input = []
 
-        for country in self.wanted_maps:
-            tiles_of_input.extend(self.get_tiles_of_wanted_map_single(country))
+        for wanted_map in self.wanted_maps:
+            try:
+                tiles_of_input.extend(
+                    self.get_tiles_of_wanted_map_single(wanted_map))
+            except XYCombinationHasNoCountries as exception:
+                # this exception is actually only raised in class XYGeofabrik
+                raise exception
 
         return tiles_of_input
 
@@ -58,6 +71,10 @@ def log_tile(self, counter, all):
         log.info(
             '(+ tile %s of %s) Find needed countries ', counter, all)
 
+    def split_input_to_list(input_value) -> list:
+        """split the input to single values in a list"""
+        pass
+
 
 class CountryGeofabrik(InformalGeofabrikInterface):
     """Geofabrik processing for countries"""
@@ -71,11 +88,7 @@ def __init__(self, input_countries):
         self.wanted_maps = []
 
         # input parameters
-        input_countries = get_countries_from_input(input_countries)
-
-        for country in input_countries:
-            self.wanted_maps.append(self.o_geofabrik_json.translate_id_no_to_geofabrik(
-                country))
+        self.wanted_maps = self.split_input_to_list(input_countries)
 
     def get_tiles_of_wanted_map_single(self, wanted_map):
         """Overrides InformalGeofabrikInterface.get_tiles_of_wanted_map_single()"""
@@ -257,6 +270,26 @@ def compose_bouding_box(self, bounds):
 
         return {'top_x': top_x, 'top_y': top_y, 'bot_x': bot_x, 'bot_y': bot_y}
 
+    @staticmethod
+    def split_input_to_list(input_value) -> list:
+        """
+        extract/split x/y combinations by given X/Y coordinates.
+        input should be "188/88" or for multiple values "188/88,100/10,109/99".
+        returns a list of x/y combinations as integers
+        """
+        countries = []
+        o_geofabrik_json = GeofabrikJson()
+
+        try:
+            # split by "," first for multiple x/y combinations, then by "/" for x and y value
+            for country in input_value.split(","):
+                countries.append(o_geofabrik_json.translate_id_no_to_geofabrik(
+                    country))
+        except CountyIsNoGeofabrikCountry as exception:
+            raise exception
+
+        return countries
+
 
 class XYGeofabrik(InformalGeofabrikInterface):
     """Geofabrik processing for X/Y coordinates"""
@@ -269,7 +302,7 @@ def __init__(self, input_xy_coordinates):
         self.o_geofabrik_json = GeofabrikJson()
 
         # use Geofabrik-URL to get the relevant tiles
-        self.wanted_maps = get_xy_coordinates_from_input(input_xy_coordinates)
+        self.wanted_maps = self.split_input_to_list(input_xy_coordinates)
 
     def get_tiles_of_wanted_map_single(self, wanted_map):
         """Overrides InformalGeofabrikInterface.get_tiles_of_wanted_map_single()"""
@@ -286,6 +319,10 @@ def get_tiles_of_wanted_map_single(self, wanted_map):
         tiles_of_input = self.find_needed_countries(
             bbox_tiles, wanted_map, wanted_region)
 
+        if len(tiles_of_input[0]['countries']) == 0:
+            raise XYCombinationHasNoCountries(
+                f'{wanted_map["x"]}/{wanted_map["y"]}')
+
         return tiles_of_input
 
     def find_needed_countries(self, bbox_tiles, wanted_map, wanted_region_polygon) -> dict:
@@ -370,6 +407,25 @@ def compose_shape(self, bbox_tiles):
         wanted_region = shape(coords_polygon)
         return wanted_region
 
+    @staticmethod
+    def split_input_to_list(input_value) -> list:
+        """
+        extract/split x/y combinations by given X/Y coordinates.
+        input should be "188/88" or for multiple values "188/88,100/10,109/99".
+        returns a list of x/y combinations as integers
+        """
+        xy_combinations = []
+
+        # split by "," first for multiple x/y combinations, then by "/" for x and y value
+        for xy_coordinate in input_value.split(","):
+            splitted = xy_coordinate.split("/")
+
+            if len(splitted) == 2:
+                xy_combinations.append(
+                    {"x": int(splitted[0]), "y": int(splitted[1])})
+
+        return xy_combinations
+
 
 def calc_bounding_box_tiles(bbox):
     bbox_tiles = []
@@ -420,39 +476,3 @@ def num2deg(xtile, ytile, zoom=8):
     lat_rad = math.atan(math.sinh(math.pi * (1 - 2 * ytile / n)))
     lat_deg = math.degrees(lat_rad)
     return (lat_deg, lon_deg)
-
-
-def get_countries_from_input(input_countries):
-    """
-    extract/split x/y combinations by given X/Y coordinates.
-    input should be "188/88" or for multiple values "188/88,100/10,109/99".
-    returns a list of x/y combinations as integers
-    """
-
-    countries = []
-
-    # split by "," first for multiple x/y combinations, then by "/" for x and y value
-    for country in input_countries.split(","):
-        countries.append(country)
-
-    return countries
-
-
-def get_xy_coordinates_from_input(input_xy_coordinates):
-    """
-    extract/split x/y combinations by given X/Y coordinates.
-    input should be "188/88" or for multiple values "188/88,100/10,109/99".
-    returns a list of x/y combinations as integers
-    """
-
-    xy_combinations = []
-
-    # split by "," first for multiple x/y combinations, then by "/" for x and y value
-    for xy_coordinate in input_xy_coordinates.split(","):
-        splitted = xy_coordinate.split("/")
-
-        if len(splitted) == 2:
-            xy_combinations.append(
-                {"x": int(splitted[0]), "y": int(splitted[1])})
-
-    return xy_combinations