[DEV] correct pylint findings (#103)

* pylint config adjustments - lint with pylint instead of flake8 - ignore line-too-long finding - only one "disable" line in .pylintrc * pylint: ignore files * tackle pylint findings * change run_subprocess function - introduced with #101, commit 1f7d340
treee111 · Apr 9, 2022 · cd8be6d · cd8be6d
1 parent 6026893
commit cd8be6d
Show file tree

Hide file tree

Showing 9 changed files with 203 additions and 209 deletions.
diff --git a/.pylintrc b/.pylintrc
@@ -9,7 +9,14 @@
 # can either give multiple identifier separated by comma (,) or put this option
 # multiple time (only on the command line, not in the configuration file where
 # it should appear only once).
-# disable=line-too-long
+disable=line-too-long, import-error
 
 #import-error, wrong-import-position
-disable=import-error
+#disable=import-error
+
+; [MASTER]
+; init-hook='import sys; sys.path.append("/path/to/root")'
+; init-hook="from pylint.config import find_pylintrc; import os, sys; sys.path.append(os.path.dirname(find_pylintrc()))"
+
+[MASTER]
+init-hook="from pylint.config import find_pylintrc; import os, sys; sys.path.append(os.path.dirname(find_pylintrc()))"
diff --git a/common_python/file_directory_functions.py b/common_python/file_directory_functions.py
@@ -57,7 +57,8 @@ def unzip(source_filename, dest_dir):
             for word in words[:-1]:
                 while True:
                     drive, word = os.path.splitdrive(word)
-                    head, word = os.path.split(word)
+                    head, word = os.path.split(
+                        word)  # pylint: disable=unused-variable
                     if not drive:
                         break
                 if word in (os.curdir, os.pardir, ''):

diff --git a/common_python/geofabrik.py b/common_python/geofabrik.py
@@ -2,6 +2,7 @@
 functions and object for managing OSM maps
 """
 #!/usr/bin/python
+#pylint: skip-file
 
 # import official python packages
 import sys
@@ -206,7 +207,7 @@ def find_needed_countries(bbox_tiles, wanted_map, wanted_region_polygon):
         # Do progress indicator every 50 tiles
         if counter % 50 == 0:
             log.info(
-                'Processing tile %s of %s',counter, len(bbox_tiles)+1)
+                'Processing tile %s of %s', counter, len(bbox_tiles)+1)
         counter += 1
 
         parent_added = 0

diff --git a/common_python/input.py b/common_python/input.py
@@ -29,7 +29,7 @@ def process_call_of_the_tool():
                                            help='sub-command help', dest='subparser_name')
 
     # create the parser for the "gui" command
-    parser_gui = subparsers.add_parser(
+    parser_gui = subparsers.add_parser(  # pylint: disable=unused-variable
         'gui', help='Start graphical user interface to select options')
 
     # create the parser for the "cli" command
@@ -203,8 +203,8 @@ def start_gui(self):
         # start GUI
         self.mainloop()
 
-        if self.o_input_data.is_required_input_given_or_exit(issue_message=False):
-            return self.o_input_data
+        self.o_input_data.is_required_input_given_or_exit(issue_message=True)
+        return self.o_input_data
 
     def build_gui(self):
         """
@@ -244,7 +244,7 @@ def build_gui(self):
             tab2, self.o_input_data)
         tab2.first.pack(side=tk.TOP, fill=tk.X)
 
-    def handle_create_map(self, event):
+    def handle_create_map(self, event):  # pylint: disable=unused-argument
         """
         run when Button "Create" is pressed
         """
@@ -273,7 +273,7 @@ def handle_create_map(self, event):
 
         self.destroy()
 
-    def switch_reload(self, event):
+    def switch_reload(self, event):  # pylint: disable=unused-argument
         """
         switch edit-mode of max-days field
         """
@@ -329,7 +329,7 @@ def __init__(self, parent, oInputData):
             column=0, row=3, sticky=tk.E, padx=5, pady=2)
         self.en_max_days_old.grid(column=1, row=3, sticky=tk.W, padx=10)
 
-    def callback_continent(self, event):
+    def callback_continent(self, event):  # pylint: disable=unused-argument
         """
         set value-list of countries after changing continent
         """

diff --git a/common_python/osm_maps_functions.py b/common_python/osm_maps_functions.py
@@ -65,24 +65,22 @@ def get_tile_by_one_xy_combination_from_jsons(xy_combination):
                     return tile
 
 
-def log_subprocess_output(pipe):
-    for line in iter(pipe.readline, b''):  # b'\n'-separated lines
-        log.info('subprocess:%r', line.decode("utf-8").strip())
-
-
-def run_subprocess_and_log_output(cmd, cwd=""):
+def run_subprocess_and_log_output(cmd, error_message, cwd=""):
+    """
+    run given cmd-subprocess and issue error message if wished
+    """
     if not cwd:
-        process = subprocess.Popen(
-            cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+        with subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) as process:
+            for line in iter(process.stdout.readline, b''):  # b'\n'-separated lines
+                log.debug('subprocess:%r', line.decode("utf-8").strip())
     else:
-        process = subprocess.Popen(
-            cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, cwd=cwd)
+        with subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, cwd=cwd) as process:
+            for line in iter(process.stdout.readline, b''):  # b'\n'-separated lines
+                log.debug('subprocess:%r', line.decode("utf-8").strip())
 
-    with process.stdout:
-        # log_subprocess_output(process.stdout)
-        for line in iter(process.stdout.readline, b''):  # b'\n'-separated lines
-            log.debug('subprocess:%r', line.decode("utf-8").strip())
-    return process.wait()  # 0 means success
+    if error_message and process.wait() != 0:  # 0 means success
+        log.error(error_message)
+        sys.exit()
 
 
 class OsmMaps:
@@ -252,11 +250,8 @@ def filter_tags_from_country_osm_pbf_files(self):
                     cmd.append(val['map_file'])
                     cmd.append('-o='+out_file_o5m)
 
-                    result = run_subprocess_and_log_output(cmd)
-                    if result != 0:
-                        log.error(
-                            '! Error in OSMConvert with country: %s', key)
-                        sys.exit()
+                    run_subprocess_and_log_output(
+                        cmd, '! Error in OSMConvert with country: {key}')
 
                     log.info(
                         '+ Filtering unwanted map objects out of map of %s', key)
@@ -267,10 +262,8 @@ def filter_tags_from_country_osm_pbf_files(self):
                                constants.FILTERED_TAGS_WIN + '"')
                     cmd.append('-o=' + out_file_o5m_filtered)
 
-                    result = run_subprocess_and_log_output(cmd)
-                    if result != 0:
-                        log.error('! Error in OSMFilter with country: %s', key)
-                        sys.exit()
+                    run_subprocess_and_log_output(
+                        cmd, '! Error in OSMFilter with country: {key}')
 
                     cmd = [os.path.join(fd_fct.TOOLING_WIN_DIR, 'osmfilter')]
                     cmd.append(out_file_o5m)
@@ -280,10 +273,8 @@ def filter_tags_from_country_osm_pbf_files(self):
                                constants.FILTERED_TAGS_WIN_NAMES + '"')
                     cmd.append('-o=' + out_file_o5m_filtered_names)
 
-                    result = run_subprocess_and_log_output(cmd)
-                    if result != 0:
-                        log.error('! Error in OSMFilter with country: %s', key)
-                        sys.exit()
+                    run_subprocess_and_log_output(
+                        cmd, '! Error in OSMFilter with country: {key}')
 
                     os.remove(out_file_o5m)
 
@@ -307,21 +298,17 @@ def filter_tags_from_country_osm_pbf_files(self):
                     cmd.extend(['-o', out_file_o5m_filtered])
                     cmd.append('--overwrite')
 
-                    result = run_subprocess_and_log_output(cmd)
-                    if result != 0:
-                        log.error('! Error in Osmium with country: %s', key)
-                        sys.exit()
+                    run_subprocess_and_log_output(
+                        cmd, '! Error in Osmium with country: {key}')
 
                     cmd = ['osmium', 'tags-filter', '--remove-tags']
                     cmd.append(val['map_file'])
                     cmd.extend(constants.FILTERED_TAGS_NAMES)
                     cmd.extend(['-o', out_file_o5m_filtered_names])
                     cmd.append('--overwrite')
 
-                    result = run_subprocess_and_log_output(cmd)
-                    if result != 0:
-                        log.error('! Error in Osmium with country: %s', key)
-                        sys.exit()
+                    run_subprocess_and_log_output(
+                        cmd, '! Error in Osmium with country: {key}')
 
                 val['filtered_file'] = out_file_o5m_filtered
                 val['filtered_file_names'] = out_file_o5m_filtered_names
@@ -362,7 +349,8 @@ def generate_land(self):
                 cmd.append(land_file)
                 cmd.append(fd_fct.LAND_POLYGONS_PATH)
 
-                result = run_subprocess_and_log_output(cmd)
+                run_subprocess_and_log_output(
+                    cmd, f'! Error generating land for tile: {tile["x"]},{tile["y"]}')
 
             # create land1.osm
             if not os.path.isfile(out_file+'1.osm') or self.force_processing is True:
@@ -376,7 +364,8 @@ def generate_land(self):
                     cmd = ['python3', os.path.join(fd_fct.COMMON_DIR,
                                                    'shape2osm.py'), '-l', out_file, land_file]
 
-                result = run_subprocess_and_log_output(cmd)
+                run_subprocess_and_log_output(
+                    cmd, f'! Error creating land.osm for tile: {tile["x"]},{tile["y"]}')
             tile_count += 1
 
         log.info('+ Generate land: OK')
@@ -456,11 +445,8 @@ def split_filtered_country_files_to_tiles(self):
                         cmd.append(val['filtered_file'])
                         cmd.append('-o='+out_file)
 
-                        result = run_subprocess_and_log_output(cmd)
-                        if result != 0:
-                            log.error(
-                                '!  Error in Osmosis with country: %s', country)
-                            sys.exit()
+                        run_subprocess_and_log_output(
+                            cmd, f'! Error in Osmosis with country: {country}')
 
                         cmd = [self.osmconvert_path,
                                '-v', '--hash-memory=2500']
@@ -471,11 +457,8 @@ def split_filtered_country_files_to_tiles(self):
                         cmd.append(val['filtered_file_names'])
                         cmd.append('-o='+out_file_names)
 
-                        result = run_subprocess_and_log_output(cmd)
-                        if result != 0:
-                            log.error(
-                                '! Error in Osmosis with country: %s', country)
-                            sys.exit()
+                        run_subprocess_and_log_output(
+                            cmd, '! Error in Osmosis with country: {country}')
 
                     # Non-Windows
                     else:
@@ -487,11 +470,8 @@ def split_filtered_country_files_to_tiles(self):
                         cmd.extend(['-o', out_file])
                         cmd.extend(['--overwrite'])
 
-                        result = run_subprocess_and_log_output(cmd)
-                        if result != 0:
-                            log.error(
-                                '! Error in Osmosis with country: %s', country)
-                            sys.exit()
+                        run_subprocess_and_log_output(
+                            cmd, '! Error in Osmosis with country: {country}')
 
                         cmd = ['osmium', 'extract']
                         cmd.extend(
@@ -501,11 +481,8 @@ def split_filtered_country_files_to_tiles(self):
                         cmd.extend(['-o', out_file_names])
                         cmd.extend(['--overwrite'])
 
-                        result = run_subprocess_and_log_output(cmd)
-                        if result != 0:
-                            log.error(
-                                '! Error in Osmosis with country: %s', country)
-                            sys.exit()
+                        run_subprocess_and_log_output(
+                            cmd, '! Error in Osmosis with country: {country}')
 
                         log.info(val['filtered_file'])
 
@@ -583,11 +560,8 @@ def merge_splitted_tiles_with_land_and_sea(self, calc_border_countries):
                     cmd.append(os.path.join(out_tile_dir, 'sea.osm'))
                     cmd.extend(['-o', out_file])
 
-                result = run_subprocess_and_log_output(cmd)
-                if result != 0:
-                    log.error(
-                        '! Error in Osmosis with tile: %s,%s', tile["x"], tile["y"])
-                    sys.exit()
+                run_subprocess_and_log_output(
+                    cmd, f'! Error in Osmosis with tile: {tile["x"]},{tile["y"]}')
 
             tile_count += 1
 
@@ -623,11 +597,8 @@ def sort_osm_files(self, tile):
                 cmd.append(land)
                 cmd.extend(['-o', land])
 
-        result = run_subprocess_and_log_output(cmd)
-        if result != 0:
-            log.error(
-                '! Error in Osmosis with sorting land* osm files of tile: %s,%s', tile["x"], tile["y"])
-            sys.exit()
+        run_subprocess_and_log_output(
+            cmd, f'Error in Osmosis with sorting land* osm files of tile: {tile["x"]},{tile["y"]}')
 
         log.info('+ Sorting land* osm files: OK')
 
@@ -671,11 +642,8 @@ def create_map_files(self, save_cruiser, tag_wahoo_xml):
                 cmd.append(
                     f'tag-conf-file={os.path.join(fd_fct.COMMON_DIR, "tag_wahoo_adjusted", tag_wahoo_xml)}')
 
-                result = run_subprocess_and_log_output(cmd)
-                if result != 0:
-                    log.error(
-                        '! Error in Osmosis with country: c // tile: %s,%s', tile["x"], tile["y"])
-                    sys.exit()
+                run_subprocess_and_log_output(
+                    cmd, f'Error in Osmosis with country: c // tile: {tile["x"]},{tile["y"]}')
 
                 # Windows
                 if platform.system() == "Windows":
@@ -690,7 +658,8 @@ def create_map_files(self, save_cruiser, tag_wahoo_xml):
                     if save_cruiser:
                         cmd.append('--keep')
 
-                result = run_subprocess_and_log_output(cmd)
+                run_subprocess_and_log_output(
+                    cmd, f'! Error creating map files for tile: {tile["x"]},{tile["y"]}')
 
             # Create "tile present" file
             with open(out_file + '.lzma.12', 'wb') as tile_present_file:
@@ -768,7 +737,8 @@ def make_and_zip_files(self, keep_map_folders, extension):
             cmd.extend(
                 [folder_name + '.zip', folder_name])
 
-        result = run_subprocess_and_log_output(cmd, cwd=fd_fct.OUTPUT_DIR)
+        run_subprocess_and_log_output(
+            cmd, f'! Error zipping map files for folder: {folder_name}', cwd=fd_fct.OUTPUT_DIR)
 
         # Keep (True) or delete (False) the country/region map folders after compression
         if keep_map_folders is False:

diff --git a/common_resources/shape2osm.py b/common_resources/shape2osm.py
@@ -1,4 +1,5 @@
 #!/usr/bin/python3
+# pylint: skip-file
 
 """
 This script is designed to act as assistance in converting shapefiles

diff --git a/tests/test_downloader.py b/tests/test_downloader.py
@@ -74,12 +74,14 @@ def test_future_timestamp(self):
 
     def test_download_geofabrik_file(self):
         """
-        Test the download of land poligons file via URL
+        Test the download of geofabrik file via URL
         """
         path = os.path.join(fd_fct.COMMON_DL_DIR, 'geofabrik.json')
         download_file(
             path, 'https://download.geofabrik.de/index-v1.json', False)
 
+        self.assertTrue(os.path.exists(path))
+
 
 if __name__ == '__main__':
     unittest.main()