Bugfix #2179 develop TCPairs fix -diag argument

internal/tests/pytests/util/config/test_config.py

@@ -72,7 +72,7 @@ def test_getstr(metplus_config, input_value, default, result):
         ('/some/test/dir', None, '/some/test/dir'),
         ('/some//test/dir', None, '/some/test/dir'),
         ('/path/to', None, 'ValueError'),
-        (None, None, 'NoOptionError'),
+        (None, None, ''),
         (None, '/default/path', '/default/path'),
 
     ]
@@ -83,11 +83,9 @@ def test_getdir(metplus_config, input_value, default, result):
     if input_value is not None:
         conf.set('config', 'TEST_GETDIR', input_value)
 
-    # catch NoOptionError exception and pass test if default is None
+    # catch ValueError and pass test if value is invalid and no default
     try:
         assert result == conf.getdir('TEST_GETDIR', default=default)
-    except NoOptionError:
-        assert result == 'NoOptionError'
     except ValueError:
         assert result == 'ValueError'
 

internal/tests/pytests/wrappers/tc_pairs/test_tc_pairs_wrapper.py

@@ -78,6 +78,7 @@ def test_read_storm_info(metplus_config, config_overrides, isOK):
     wrapper = TCPairsWrapper(config)
     assert wrapper.isOK == isOK
 
+
 @pytest.mark.parametrize(
     'storm_id,basin,cyclone', [
         ('AL092011', 'al', '09'),
@@ -540,6 +541,23 @@ def test_tc_pairs_storm_id_lists(metplus_config, config_overrides,
                  'field_source = "GFS_0p50";match_to_track = ["OFCL"]'
                  ';diag_name = ["MY_NAME2"];}];'
          )}),
+        # -diag arguments with multiple models
+        ('VALID', {
+            'MODEL': 'MYNN, H19C',
+            'TC_PAIRS_DIAG_TEMPLATE1': '/some/path/{valid?fmt=%Y%m%d%H}_{model}.dat',
+            'TC_PAIRS_DIAG_SOURCE1': 'TCDIAG',
+            'TC_PAIRS_DIAG_TEMPLATE2': '/some/path/rt_{valid?fmt=%Y%m%d%H}.dat',
+            'TC_PAIRS_DIAG_SOURCE2': 'LSDIAG_RT',
+        },
+         {'DIAG_ARG': ('-diag TCDIAG /some/path/2014121318_MYNN.dat /some/path/2014121318_H19C.dat '
+                       '-diag LSDIAG_RT /some/path/rt_2014121318.dat'),
+          'METPLUS_MODEL': 'model = ["MYNN", "H19C"];'}),
+        # -diag arguments with wildcard model
+        ('VALID', {
+            'TC_PAIRS_DIAG_TEMPLATE1': 'bmlq2014123118.{model}.0104',
+            'TC_PAIRS_DIAG_SOURCE1': 'TCDIAG',
+        },
+         {'DIAG_ARG': '-diag TCDIAG <BDECK_DIR>/bmlq2014123118.gfso.0104',}),
     ]
 )
 @pytest.mark.wrapper
@@ -556,6 +574,8 @@ def test_tc_pairs_run(metplus_config, loop_by, config_overrides,
 
     config.set('config', 'TC_PAIRS_BDECK_INPUT_DIR', bdeck_dir)
     config.set('config', 'TC_PAIRS_ADECK_INPUT_DIR', adeck_dir)
+    if 'DIAG_ARG' in env_var_values and 'TC_PAIRS_DIAG_DIR1' not in config_overrides:
+        config.set('config', 'TC_PAIRS_DIAG_DIR1', bdeck_dir)
 
     # set config variable overrides
     for key, value in config_overrides.items():
@@ -589,6 +609,7 @@ def test_tc_pairs_run(metplus_config, loop_by, config_overrides,
     diag_arg = ''
     if 'DIAG_ARG' in env_var_values:
         diag_arg = f" {env_var_values['DIAG_ARG']}"
+        diag_arg = diag_arg.replace('<BDECK_DIR>', bdeck_dir)
 
     expected_cmds = [(f"{app_path} {verbosity} "
                       f"-bdeck {bdeck_dir}/bmlq2014123118.gfso.0104 "

metplus/util/config_metplus.py

@@ -530,10 +530,12 @@ def getraw(self, sec, opt, default='', count=0, sub_vars=True):
             config, dir, and os environment)
             returns raw string, preserving {valid?fmt=%Y} blocks
 
-            @param sec: Section in the conf file to look for variable
-            @param opt: Variable to interpret
-            @param default: Default value to use if config is not set
-            @param count: Counter used to stop recursion to prevent infinite
+            @param sec Section in the conf file to look for variable
+            @param opt Variable to interpret
+            @param default Default value to use if config is not set
+            @param count Counter used to stop recursion to prevent infinite
+            @param sub_vars If False, skip string template substitution,
+             defaults to True
             @returns Raw string or empty string if function calls itself too
              many times
         """
@@ -640,17 +642,11 @@ def getexe(self, exe_name):
         self.set('config', exe_name, full_exe_path)
         return full_exe_path
 
-    def getdir(self, dir_name, default=None, morevars=None,taskvars=None,
-               must_exist=False):
+    def getdir(self, dir_name, default=None, must_exist=False):
         """! Wraps produtil getdir and reports an error if
          it is set to /path/to
          """
-        try:
-            dir_path = super().getstr('config', dir_name, default=None,
-                                      morevars=morevars, taskvars=taskvars)
-        except NoOptionError:
-            self.check_default('config', dir_name, default)
-            dir_path = default
+        dir_path = self.getraw('config', dir_name, default=default)
 
         if '/path/to' in dir_path:
             raise ValueError(f"{dir_name} cannot be set to "

metplus/wrappers/command_builder.py

@@ -566,13 +566,11 @@ def find_exact_file(self, level, data_type, time_info, mandatory=True,
         input_must_exist = self.c_dict.get('INPUT_MUST_EXIST', True)
 
         for template in template_list:
-            # perform string substitution
-            filename = do_string_sub(template,
-                                     level=level,
-                                     **time_info)
-
-            # build full path with data directory and filename
-            full_path = os.path.join(data_dir, filename)
+            # perform string substitution on directory and template
+            full_template = os.path.join(data_dir, template)
+            full_path = do_string_sub(full_template,
+                                      level=level,
+                                      **time_info)
 
             if os.path.sep not in full_path:
                 self.logger.debug(f"{full_path} is not a file path. "
@@ -690,6 +688,9 @@ def find_file_in_window(self, level, data_type, time_info, mandatory=True,
             self.log_error('Must set INPUT_DIR if looking for files within a time window')
             return None
 
+        # substitute any filename template tags that may be in _INPUT_DIR
+        data_dir = do_string_sub(data_dir, **time_info)
+
         # step through all files under input directory in sorted order
         for dirpath, _, all_files in os.walk(data_dir):
             for filename in sorted(all_files):
@@ -941,8 +942,7 @@ def find_and_check_output_file(self, time_info=None,
 
         # substitute time info if provided
         if time_info:
-            output_path = do_string_sub(output_path,
-                                        **time_info)
+            output_path = do_string_sub(output_path, **time_info)
 
         # replace wildcard character * with all
         output_path = output_path.replace('*', 'all')

metplus/wrappers/tc_pairs_wrapper.py

@@ -474,6 +474,10 @@ def _handle_diag(self, c_dict):
             source = (
                 self.config.getraw('config', f'TC_PAIRS_DIAG_SOURCE{idx}')
             )
+            if not source:
+                self.log_error(f'TC_PAIRS_DIAG_SOURCE{idx} must be set if '
+                               f'TC_PAIRS_DIAG_TEMPLATE{idx} is set')
+                continue
             diag_info = {
                 'template': template,
                 'source': source,
@@ -658,6 +662,10 @@ def process_data(self, basin, cyclone, time_info):
             if current_basin is None or current_cyclone is None:
                 continue
 
+            time_storm_info = time_info.copy()
+            time_storm_info['basin'] = current_basin
+            time_storm_info['cyclone'] = current_cyclone
+
             # create lists for deck files, put bdeck in list so it can
             # be handled the same as a and e for reformatting even though
             # it will always be size 1
@@ -667,14 +675,10 @@ def process_data(self, basin, cyclone, time_info):
 
             # get adeck files
             if self.c_dict['GET_ADECK']:
-                adeck_list = self.find_a_or_e_deck_files('A', current_basin,
-                                                         current_cyclone,
-                                                         time_info)
+                adeck_list = self.find_a_or_e_deck_files('A', time_storm_info)
             # get edeck files
             if self.c_dict['GET_EDECK']:
-                edeck_list = self.find_a_or_e_deck_files('E', current_basin,
-                                                         current_cyclone,
-                                                         time_info)
+                edeck_list = self.find_a_or_e_deck_files('E', time_storm_info)
 
             if not adeck_list and not edeck_list:
                 self.log_error('Could not find any corresponding '
@@ -693,20 +697,16 @@ def process_data(self, basin, cyclone, time_info):
             if edeck_list:
                 self.args.append(f"-edeck {' '.join(edeck_list)}")
 
-            # change wildcard basin/cyclone to 'all' for output filename
-            if current_basin == self.WILDCARDS['basin']:
-                current_basin = 'all'
-            if current_cyclone == self.WILDCARDS['cyclone']:
-                current_cyclone = 'all'
-
-            time_storm_info = time_info.copy()
-            time_storm_info['basin'] = current_basin
-            time_storm_info['cyclone'] = current_cyclone
-
             # find -diag file if requested
             if not self._get_diag_file(time_storm_info):
                 return []
 
+            # change wildcard basin/cyclone to 'all' for output filename
+            if current_basin == self.WILDCARDS['basin']:
+                time_storm_info['basin'] = 'all'
+            if current_cyclone == self.WILDCARDS['cyclone']:
+                time_storm_info['cyclone'] = 'all'
+
             if not self.find_and_check_output_file(time_info=time_storm_info,
                                                    check_extension='.tcst'):
                 return []
@@ -811,22 +811,18 @@ def _get_basin_cyclone_from_bdeck(self, bdeck_file, wildcard_used,
 
         return current_basin, current_cyclone
 
-    def find_a_or_e_deck_files(self, deck, basin, cyclone, time_info):
+    def find_a_or_e_deck_files(self, deck, time_info):
         """!Find ADECK or EDECK files that correspond to the BDECk file found
 
             @param deck type of deck (A or E)
-            @param basin region of storm from config
-            @param cyclone ID number of cyclone from config
-            @param time_info dictionary with timing info for current run
+            @param time_info dictionary with timing/storm info for current run
         """
         deck_list = []
         template = os.path.join(self.c_dict[deck+'DECK_DIR'],
                                 self.c_dict[deck+'DECK_TEMPLATE'])
 
         # get matching adeck wildcard expression for first model
         deck_expr = do_string_sub(template,
-                                  basin=basin,
-                                  cyclone=cyclone,
                                   model=self.c_dict['MODEL_LIST'][0],
                                   **time_info)
 
@@ -899,16 +895,18 @@ def get_command(self):
                f" -out {output_path}")
         return cmd
 
-    def read_modify_write_file(self, in_csvfile, storm_month, missing_values,
+    @staticmethod
+    def read_modify_write_file(in_csvfile, storm_month, missing_values,
                                out_csvfile):
-        """! Reads, modifies and writes file
-              Args:
-                @param in_csvfile input csv file that is being parsed
-                @param storm_month The storm month
-                @param missing_values a tuple where (MISSING_VAL_TO_REPLACE,
-                                                     MISSING_VAL)
-                @param out_csvfile the output csv file
-                @param logger the log where logging is directed
+        """!Reads CSV file, reformat file by adding the month to the 2nd
+        column storm number, delete the 3rd column, replace missing values,
+        and write a new CSV file with the modified content.
+
+        @param in_csvfile input csv file that is being parsed
+        @param storm_month storm month to prepend to storm number
+        @param missing_values tuple containing a missing data value to find in
+        the columns and the value to replace it with, e.g. (-9, -9999)
+        @param out_csvfile the output csv file
         """
         # create output directory if it does not exist
         mkdir_p(os.path.dirname(out_csvfile))
@@ -943,15 +941,14 @@ def read_modify_write_file(self, in_csvfile, storm_month, missing_values,
                         continue
                     # Replace MISSING_VAL_TO_REPLACE=missing_values[0] with
                     # MISSING_VAL=missing_values[1]
-                    elif item.strip() == missing_values[0]:
+                    if item.strip() == missing_values[0]:
                         item = " " + missing_values[1]
                     # Create a new row to write
                     row_list.append(item)
 
                 # Write the modified file
                 writer.writerow(row_list)
 
-        csvfile.close()
         out_file.close()
 
     def _read_all_files(self, input_dict):
@@ -1005,16 +1002,26 @@ def _get_diag_file(self, time_info):
         """
         if not self.c_dict.get('DIAG_INFO_LIST'):
             return True
-        self.log_error(self.c_dict.get('DIAG_INFO_LIST'))
+
+        time_info_copy = time_info.copy()
         for diag_info in self.c_dict.get('DIAG_INFO_LIST'):
             self.c_dict['DIAG_INPUT_TEMPLATE'] = diag_info['template']
-            filepaths = self.find_data(time_info, data_type='DIAG',
-                                       return_list=True)
-            if not filepaths:
+            all_files = []
+            for model in self.c_dict['MODEL_LIST']:
+                time_info_copy['model'] = model
+                filepaths = self.find_data(time_info_copy, data_type='DIAG',
+                                           return_list=True)
+                if filepaths:
+                    all_files.extend(filepaths)
+
+            if not all_files:
                 self.log_error('Could not get -diag files')
                 return False
 
-            arg = f"-diag {diag_info['source']} {' '.join(filepaths)}"
+            # remove duplicate files
+            all_files = list(set(all_files))
+
+            arg = f"-diag {diag_info['source']} {' '.join(all_files)}"
             self.args.append(arg)
 
         return True