Feature #2758 SonarQube (1) (#2866)

* test that this file is not needed anymore because RTD controls the version selector

* various changes to appease SonarQube complaints

* remove deprecated docker command

* resolve more SonarQube code smells

* update URL for posting sample data

.github/jobs/free_disk_space.sh

@@ -59,10 +59,6 @@ cmd="docker image prune -af"
 printf "\nRunning $cmd"
 $cmd
 
-cmd=docker system prune -af
-printf "\nRunning $cmd"
-$cmd
-
 cmd="docker images"
 printf "\nAFTER CLEANUP: $cmd"
 $cmd

docs/build_docs.py

@@ -31,7 +31,7 @@ def run_command(command, dir_to_run=None):
     log_text = f"Running {command}"
     if dir_to_run:
         log_text += f" under {dir_to_run}"
-
+    print(log_text)
     command_out = subprocess.run(shlex.split(command),
                                  cwd=dir_to_run)
     if command_out.returncode != 0:
@@ -103,8 +103,7 @@ def main():
                                'run')
 
     # run make to generate the documentation files
-    run_command(f"make clean html",
-                docs_dir)
+    run_command("make clean html", docs_dir)
 
     if not skip_doxygen:
         # build the doxygen documentation
@@ -157,14 +156,14 @@ def main():
     warning_file = os.path.join(docs_dir,
                                 '_build',
                                 'warnings.log')
-    if os.stat(warning_file).st_size == 0:
-        print(f"No warnings found, removing {warning_file}")
-        os.remove(warning_file)
-    else:
+    if os.stat(warning_file).st_size != 0:
         print('ERROR: Doc build contains warnings or errors. '
               f'Please review {warning_file}')
         sys.exit(1)
 
+    print(f"No warnings found, removing {warning_file}")
+    os.remove(warning_file)
+
     print("Documentation build completed")
 
 if __name__ == "__main__":

docs/conf.py

@@ -198,7 +198,7 @@
 #
 html_theme = 'sphinx_rtd_theme'
 html_theme_path = ["_themes", ]
-html_js_files = ['pop_ver.js']
+html_js_files = []
 html_css_files = ['theme_override.css']
 
 # Add any paths that contain custom static files (such as style sheets) here,

internal/scripts/dev_tools/compile_official_release_notes.py

@@ -19,7 +19,7 @@
 items = {}
 # gather issues and organize them by category
 for line in content:
-    if match := re.match(r'  .. dropdown:: (.*)', line):
+    if match := re.match(r' {2}.. dropdown:: (.*)', line):
         category = match.group(1)
         if not items.get(category):
             items[category] = []
@@ -43,7 +43,7 @@
     if not issues.get(cat):
         issues[cat] = {}
     for issue in item_list:
-        match = re.match(r'.*\#(\d+).*', issue.replace('\n', ''))
+        match = re.match(r'.*#(\d+).*', issue.replace('\n', ''))
         if match:
             issues[cat][match.group(1)] = issue
 

internal/scripts/docker/Dockerfile

@@ -18,7 +18,7 @@ ARG MET_DOCKER_REPO=met
 ARG MET_TAG=develop
 
 # if OBTAIN_SOURCE_CODE=copy, copy files from build context into image
-FROM dtcenter/${MET_DOCKER_REPO}:${MET_TAG} as build_copy
+FROM dtcenter/${MET_DOCKER_REPO}:${MET_TAG} AS build_copy
 ONBUILD WORKDIR /metplus
 ONBUILD RUN mkdir -p METplus
 ONBUILD COPY . METplus
@@ -28,7 +28,7 @@ ONBUILD RUN if [ ! -e "METplus/ush/run_metplus.py" ]; then \
   fi
 
 # if OBTAIN_SOURCE_CODE=clone, clone repository
-FROM dtcenter/${MET_DOCKER_REPO}:${MET_TAG} as build_clone
+FROM dtcenter/${MET_DOCKER_REPO}:${MET_TAG} AS build_clone
 ONBUILD WORKDIR /metplus
 ONBUILD ARG SOURCE_VERSION
 ONBUILD RUN echo "Cloning METplus repository"; \
@@ -40,7 +40,7 @@ ONBUILD RUN echo "Cloning METplus repository"; \
   fi
 
 # if OBTAIN_SOURCE_CODE=none, do not retrieve files
-FROM dtcenter/${MET_DOCKER_REPO}:${MET_TAG} as build_none
+FROM dtcenter/${MET_DOCKER_REPO}:${MET_TAG} AS build_none
 ONBUILD WORKDIR /metplus
 
 # use build alias based on OBTAIN_SOURCE_CODE value

internal/tests/pytests/conftest.py

@@ -185,7 +185,7 @@ def make_nc(tmp_path, lon, lat, z, data, variable='Temp', file_name='fake.nc'):
         latitude = rootgrp.createVariable("Latitude", "f4", "lat")
         levels = rootgrp.createVariable("Levels", "i4", "z")
         temp = rootgrp.createVariable(variable, "f4", ("time", "lon", "lat", "z"))
-        time = rootgrp.createVariable("Time", "i4", "time")
+        rootgrp.createVariable("Time", "i4", "time")
 
         longitude[:] = lon
         latitude[:] = lat

internal/tests/pytests/util/config_metplus/test_config_metplus.py

@@ -10,6 +10,10 @@
 from metplus.util.config_validate import validate_config_variables
 
 
+var1_levels = "LEVELS11, LEVELS12"
+var2_levels = "LEVELS21, LEVELS22"
+var1_options = 'ens_ssvar_bin_size = 0.1; ens_phist_bin_size = 0.05;'
+
 @pytest.mark.parametrize(
     'config_overrides,expected_logfile', [
         (['config.LOG_METPLUS={LOG_DIR}/metplus.log'], '<LOG_DIR>/metplus.log'),
@@ -267,9 +271,9 @@ def test_get_field_config_variables_synonyms(metplus_config,
 def test_parse_var_list_fcst_only(metplus_config, data_type, list_created):
     conf = metplus_config
     conf.set('config', 'FCST_VAR1_NAME', "NAME1")
-    conf.set('config', 'FCST_VAR1_LEVELS', "LEVELS11, LEVELS12")
+    conf.set('config', 'FCST_VAR1_LEVELS', var1_levels)
     conf.set('config', 'FCST_VAR2_NAME', "NAME2")
-    conf.set('config', 'FCST_VAR2_LEVELS', "LEVELS21, LEVELS22")
+    conf.set('config', 'FCST_VAR2_LEVELS', var2_levels)
 
     # this should not occur because OBS variables are missing
     assert not validate_config_variables(conf)[0]
@@ -303,9 +307,9 @@ def test_parse_var_list_fcst_only(metplus_config, data_type, list_created):
 def test_parse_var_list_obs(metplus_config, data_type, list_created):
     conf = metplus_config
     conf.set('config', 'OBS_VAR1_NAME', "NAME1")
-    conf.set('config', 'OBS_VAR1_LEVELS', "LEVELS11, LEVELS12")
+    conf.set('config', 'OBS_VAR1_LEVELS', var1_levels)
     conf.set('config', 'OBS_VAR2_NAME', "NAME2")
-    conf.set('config', 'OBS_VAR2_LEVELS', "LEVELS21, LEVELS22")
+    conf.set('config', 'OBS_VAR2_LEVELS', var2_levels)
 
     # this should not occur because FCST variables are missing
     if validate_config_variables(conf)[0]:
@@ -340,9 +344,9 @@ def test_parse_var_list_obs(metplus_config, data_type, list_created):
 def test_parse_var_list_both(metplus_config, data_type, list_created):
     conf = metplus_config
     conf.set('config', 'BOTH_VAR1_NAME', "NAME1")
-    conf.set('config', 'BOTH_VAR1_LEVELS', "LEVELS11, LEVELS12")
+    conf.set('config', 'BOTH_VAR1_LEVELS', var1_levels)
     conf.set('config', 'BOTH_VAR2_NAME', "NAME2")
-    conf.set('config', 'BOTH_VAR2_LEVELS', "LEVELS21, LEVELS22")
+    conf.set('config', 'BOTH_VAR2_LEVELS', var2_levels)
 
     # this should not occur because BOTH variables are used
     if not validate_config_variables(conf)[0]:
@@ -472,7 +476,7 @@ def test_parse_var_list_fcst_and_obs_and_both(metplus_config, data_type, list_le
 def test_parse_var_list_fcst_only_options(metplus_config, data_type, list_len):
     conf = metplus_config
     conf.set('config', 'FCST_VAR1_NAME', "NAME1")
-    conf.set('config', 'FCST_VAR1_LEVELS', "LEVELS11, LEVELS12")
+    conf.set('config', 'FCST_VAR1_LEVELS', var1_levels)
     conf.set('config', 'FCST_VAR1_THRESH', ">1, >2")
     conf.set('config', 'OBS_VAR1_OPTIONS', "OOPTIONS11")
 
@@ -533,13 +537,11 @@ def test_parse_var_list_ensemble(metplus_config):
     config.set('config', 'FCST_VAR1_NAME', 'APCP')
     config.set('config', 'FCST_VAR1_LEVELS', 'A24')
     config.set('config', 'FCST_VAR1_THRESH', '>0.01, >=10.0')
-    config.set('config', 'FCST_VAR1_OPTIONS', ('ens_ssvar_bin_size = 0.1; '
-                                               'ens_phist_bin_size = 0.05;'))
+    config.set('config', 'FCST_VAR1_OPTIONS', var1_options)
     config.set('config', 'OBS_VAR1_NAME', 'APCP')
     config.set('config', 'OBS_VAR1_LEVELS', 'A24')
     config.set('config', 'OBS_VAR1_THRESH', '>0.01, >=10.0')
-    config.set('config', 'OBS_VAR1_OPTIONS', ('ens_ssvar_bin_size = 0.1; '
-                                              'ens_phist_bin_size = 0.05;'))
+    config.set('config', 'OBS_VAR1_OPTIONS', var1_options)
     time_info = {}
 
     expected_ens_list = [{'index': 1,
@@ -567,14 +569,11 @@ def test_parse_var_list_ensemble(metplus_config):
                           'fcst_name': 'APCP',
                           'fcst_level': 'A24',
                           'fcst_thresh': ['>0.01', '>=10.0'],
-                          'fcst_extra': ('ens_ssvar_bin_size = 0.1; '
-                                         'ens_phist_bin_size = 0.05;'),
+                          'fcst_extra': var1_options,
                           'obs_name': 'APCP',
                           'obs_level': 'A24',
                           'obs_thresh': ['>0.01', '>=10.0'],
-                          'obs_extra': ('ens_ssvar_bin_size = 0.1; '
-                                        'ens_phist_bin_size = 0.05;')
-
+                          'obs_extra': var1_options
                           },
                         ]
 

internal/tests/pytests/util/config_util/test_config_util.py

@@ -3,10 +3,9 @@
 import pytest
 
 import pprint
-import os
 from datetime import datetime
 
-from metplus.util.config_util import *
+from metplus.util import config_util as cu
 from metplus.util.config_metplus import parse_var_list
 from metplus.util.time_util import ti_calculate
 
@@ -33,7 +32,7 @@ def test_get_custom_string_list(metplus_config, conf_items, met_tool, expected_r
     for conf_key, conf_value in conf_items.items():
         config.set('config', conf_key, conf_value)
 
-    assert get_custom_string_list(config, met_tool) == expected_result
+    assert cu.get_custom_string_list(config, met_tool) == expected_result
 
 
 @pytest.mark.parametrize(
@@ -72,7 +71,7 @@ def test_get_custom_string_list(metplus_config, conf_items, met_tool, expected_r
 def test_get_process_list(metplus_config, input_list, expected_list):
     conf = metplus_config
     conf.set('config', 'PROCESS_LIST', input_list)
-    process_list = get_process_list(conf)
+    process_list = cu.get_process_list(conf)
     output_list = [item[0] for item in process_list]
     assert output_list == expected_list
 
@@ -106,7 +105,7 @@ def test_get_process_list(metplus_config, input_list, expected_list):
 def test_get_process_list_instances(metplus_config, input_list, expected_list):
     conf = metplus_config
     conf.set('config', 'PROCESS_LIST', input_list)
-    output_list = get_process_list(conf)
+    output_list = cu.get_process_list(conf)
     assert output_list == expected_list
 
 
@@ -159,11 +158,11 @@ def test_sub_var_list(metplus_config, input_dict, expected_list):
     actual_temp = parse_var_list(config)
 
     pp = pprint.PrettyPrinter()
-    print(f'Actual var list (before sub):')
+    print('Actual var list (before sub):')
     pp.pprint(actual_temp)
 
-    actual_list = sub_var_list(actual_temp, time_info)
-    print(f'Actual var list (after sub):')
+    actual_list = cu.sub_var_list(actual_temp, time_info)
+    print('Actual var list (after sub):')
     pp.pprint(actual_list)
 
     assert len(actual_list) == len(expected_list)

internal/tests/pytests/util/diff_util/test_diff_util.py

@@ -1,8 +1,6 @@
 import pytest
 
-from netCDF4 import Dataset
 import os
-import shutil
 from unittest import mock
 from PIL import Image
 import numpy as np
@@ -214,7 +212,7 @@ def test_get_file_type(path, expected):
 
 
 @pytest.mark.util
-def test_get_file_type_netCDF4(dummy_nc1):
+def test_get_file_type_netcdf4(dummy_nc1):
     actual = du.get_file_type(dummy_nc1)
     assert actual == 'netcdf'
 
@@ -488,7 +486,7 @@ def test__handle_text_files(cmp_return, comp_txt_return, expected):
 
 
 @pytest.mark.parametrize(
-    'colour_A, colour_B, save_diff, expected, check_print',
+    "colour_a, colour_b, save_diff, expected, check_print",
     [
         (
             255,
@@ -515,7 +513,7 @@ def test__handle_text_files(cmp_return, comp_txt_return, expected):
 )
 @pytest.mark.util
 def test_compare_image_files(
-    capfd, tmp_path_factory, colour_A, colour_B, save_diff, expected, check_print
+    capfd, tmp_path_factory, colour_a, colour_b, save_diff, expected, check_print
 ):
     image_dir = tmp_path_factory.mktemp('images')
     image1 = image_dir / 'img1.jpg'
@@ -528,8 +526,8 @@ def _make_test_img(file_path, col):
         im.save(file_path)
         im.close()
 
-    _make_test_img(image1, colour_A)
-    _make_test_img(image2, colour_B)
+    _make_test_img(image1, colour_a)
+    _make_test_img(image2, colour_b)
 
     actual = du.compare_image_files(image1, image2, save_diff)