From 0ad64d7d93c2bb343898b133495f4628eefb8b89 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Mon, 10 Jul 2023 16:42:03 -0600 Subject: [PATCH 1/5] per #2241, create directory containing -out_stat file to prevent MET error --- .../wrappers/tc_stat/test_tc_stat_wrapper.py | 1 + metplus/wrappers/tc_stat_wrapper.py | 24 ++++++++++++------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/internal/tests/pytests/wrappers/tc_stat/test_tc_stat_wrapper.py b/internal/tests/pytests/wrappers/tc_stat/test_tc_stat_wrapper.py index 40b66b6c1d..046f438c75 100644 --- a/internal/tests/pytests/wrappers/tc_stat/test_tc_stat_wrapper.py +++ b/internal/tests/pytests/wrappers/tc_stat/test_tc_stat_wrapper.py @@ -14,6 +14,7 @@ config_init_beg = '20170705' config_init_end = '20170901' + def get_config(metplus_config): # extra_configs = [] # extra_configs.append(os.path.join(os.path.dirname(__file__), diff --git a/metplus/wrappers/tc_stat_wrapper.py b/metplus/wrappers/tc_stat_wrapper.py index ce09ba9400..638420ba08 100755 --- a/metplus/wrappers/tc_stat_wrapper.py +++ b/metplus/wrappers/tc_stat_wrapper.py @@ -365,15 +365,9 @@ def handle_jobs(self, time_info): subbed_job = do_string_sub(job, **time_info) if time_info else job formatted_jobs.append(subbed_job.strip()) - # check if -dump_row is used - # if it is, create parent directory of output file - split_job = subbed_job.split(' ') - if '-dump_row' in split_job: - index = split_job.index('-dump_row') + 1 - filepath = split_job[index] - self.c_dict['OUTPUT_TEMPLATE'] = filepath - - if not self.find_and_check_output_file(time_info): + # create parent directory of output file + for out in ('-dump_row', '-out_stat'): + if not self._create_job_out_dirs(out, subbed_job, time_info): return None job_list_string = '","'.join(formatted_jobs) @@ -384,6 +378,18 @@ def handle_jobs(self, time_info): return job_list_string + def _create_job_out_dirs(self, out_type, job_args, time_info): + split_job = job_args.split(' ') + # return True if job arg that writes a file is not found in job args + if out_type not in split_job: + return True + + # if job arg is found, create parent directory of output file + index = split_job.index(out_type) + 1 + filepath = split_job[index] + self.c_dict['OUTPUT_TEMPLATE'] = filepath + return self.find_and_check_output_file(time_info) + def handle_out_file(self, time_info): """! If output template is set, """ From 4d4ab12ddb4bdb288afe068a9af9c904caeb3601 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Tue, 11 Jul 2023 11:54:09 -0600 Subject: [PATCH 2/5] add to list of pytest groups --- docs/Contributors_Guide/testing.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/Contributors_Guide/testing.rst b/docs/Contributors_Guide/testing.rst index e1e0af8e6a..f0a4b06f44 100644 --- a/docs/Contributors_Guide/testing.rst +++ b/docs/Contributors_Guide/testing.rst @@ -23,10 +23,12 @@ information about the local environment so that the tests can run. All unit tests must include one of the custom markers listed in the internal/tests/pytests/pytest.ini file. Some examples include: + * run_metplus * util * wrapper_a * wrapper_b * wrapper_c + * wrapper_d * wrapper * long * plotting From 8029a84a1f17534e9d2867319f841da3ae58ab4d Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Tue, 11 Jul 2023 11:54:52 -0600 Subject: [PATCH 3/5] added test to check that output template is set correctly, fixed formatting --- .../wrappers/tc_stat/test_tc_stat_wrapper.py | 72 +++++++++---------- 1 file changed, 32 insertions(+), 40 deletions(-) diff --git a/internal/tests/pytests/wrappers/tc_stat/test_tc_stat_wrapper.py b/internal/tests/pytests/wrappers/tc_stat/test_tc_stat_wrapper.py index 046f438c75..563605c9e7 100644 --- a/internal/tests/pytests/wrappers/tc_stat/test_tc_stat_wrapper.py +++ b/internal/tests/pytests/wrappers/tc_stat/test_tc_stat_wrapper.py @@ -45,17 +45,6 @@ def get_config(metplus_config): return config -def tc_stat_wrapper(metplus_config): - """! Returns a default TCStatWrapper with /path/to entries in the - metplus_system.conf and metplus_runtime.conf configuration - files. Subsequent tests can customize the final METplus configuration - to over-ride these /path/to values.""" - - # Default, empty TcStatWrapper with some configuration values set - # to /path/to: - config = get_config(metplus_config) - return TCStatWrapper(config) - @pytest.mark.parametrize( 'config_overrides, env_var_values', [ # 0: no config overrides that set env vars @@ -198,6 +187,9 @@ def tc_stat_wrapper(metplus_config): # 46 out_valid_mask ({'TC_STAT_OUT_VALID_MASK': 'MET_BASE/poly/EAST.poly', }, {'METPLUS_OUT_VALID_MASK': 'out_valid_mask = "MET_BASE/poly/EAST.poly";'}), + # 47 output template + ({'TC_STAT_OUTPUT_TEMPLATE': 'tc_stat.out.nc', }, + {}), ] ) @@ -233,7 +225,7 @@ def test_tc_stat_run(metplus_config, config_overrides, env_var_values): verbosity = f"-v {wrapper.c_dict['VERBOSITY']}" config_file = wrapper.c_dict.get('CONFIG_FILE') lookin_dir = wrapper.c_dict.get('LOOKIN_DIR') - out_temp = wrapper.c_dict.get('OUTPUT_TEMPLATE') + out_temp = wrapper.c_dict.get('JOB_OUTPUT_TEMPLATE') out_dir = wrapper.c_dict.get('OUTPUT_DIR') out_arg = f' -out {out_dir}/{out_temp}' if out_temp else '' @@ -361,24 +353,23 @@ def test_override_config_in_c_dict(metplus_config, overrides, c_dict): @pytest.mark.parametrize( 'jobs, init_dt, expected_output', [ # single fake job - (['job1'], - None, - 'jobs = ["job1"];' - ), + (['job1'], + None, + 'jobs = ["job1"];' + ), # 2 jobs, no time info - (['-job filter -dump_row /filt.tcst', - '-job rirw -line_type TCMPR '], - None, - 'jobs = ["-job filter -dump_row /filt.tcst",' - '"-job rirw -line_type TCMPR"];' - ), - + (['-job filter -dump_row /filt.tcst', + '-job rirw -line_type TCMPR '], + None, + 'jobs = ["-job filter -dump_row /filt.tcst",' + '"-job rirw -line_type TCMPR"];' + ), # 2 jobs, time info sub (['-job filter -dump_row /{init?fmt=%Y%m%d%H}.tcst', '-job rirw -line_type TCMPR '], - datetime.datetime(2019, 10, 31, 12), - 'jobs = ["-job filter -dump_row /2019103112.tcst",' - '"-job rirw -line_type TCMPR"];' + datetime.datetime(2019, 10, 31, 12), + 'jobs = ["-job filter -dump_row /2019103112.tcst",' + '"-job rirw -line_type TCMPR"];' ), ] ) @@ -389,7 +380,8 @@ def test_handle_jobs(metplus_config, jobs, init_dt, expected_output): else: time_info = None - wrapper = tc_stat_wrapper(metplus_config) + config = get_config(metplus_config) + wrapper = TCStatWrapper(config) output_base = wrapper.config.getdir('OUTPUT_BASE') output_dir = os.path.join(output_base, 'test_handle_jobs') @@ -412,19 +404,19 @@ def cleanup_test_dirs(parent_dirs, output_dir): @pytest.mark.parametrize( 'jobs, init_dt, expected_output, parent_dirs', [ # single fake job, no parent dir - (['job1'], - None, - 'jobs = ["job1"];', - None - ), + (['job1'], + None, + 'jobs = ["job1"];', + None + ), # 2 jobs, no time info, 1 parent dir - (['-job filter -dump_row /filt.tcst', - '-job rirw -line_type TCMPR '], - None, - 'jobs = ["-job filter -dump_row /filt.tcst",' - '"-job rirw -line_type TCMPR"];', - [''], - ), + (['-job filter -dump_row /filt.tcst', + '-job rirw -line_type TCMPR '], + None, + 'jobs = ["-job filter -dump_row /filt.tcst",' + '"-job rirw -line_type TCMPR"];', + [''], + ), # 2 jobs, time info sub, 1 parent dir (['-job filter -dump_row /{init?fmt=%Y%m%d%H}.tcst', @@ -453,7 +445,7 @@ def cleanup_test_dirs(parent_dirs, output_dir): 'jobs = ["-job filter -dump_row /sub1/2019103112.tcst",' '"-job filter -dump_row /sub2/20191031.tcst"];', ['/sub1', - '/sub2',], + '/sub2', ], ), ] ) From be6ce8b5c9557c2827bbfd7f7a39e8c75cb2d2cc Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Tue, 11 Jul 2023 12:14:20 -0600 Subject: [PATCH 4/5] per #2241, added unit tests to ensure directories for -out_stat and -dump_row output files are created --- .../wrappers/tc_stat/test_tc_stat_wrapper.py | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/internal/tests/pytests/wrappers/tc_stat/test_tc_stat_wrapper.py b/internal/tests/pytests/wrappers/tc_stat/test_tc_stat_wrapper.py index 563605c9e7..12bcc44db5 100644 --- a/internal/tests/pytests/wrappers/tc_stat/test_tc_stat_wrapper.py +++ b/internal/tests/pytests/wrappers/tc_stat/test_tc_stat_wrapper.py @@ -45,6 +45,57 @@ def get_config(metplus_config): return config +@pytest.mark.parametrize( + 'config_overrides, expected_dirs, expected_job_string', [ + # 0: dump_row and out_stat files + ({'TC_STAT_JOB_ARGS': ("-job summary -line_type TCMPR -column " + "'ABS(AMAX_WIND-BMAX_WIND)' " + "-dump_row dump_row/summary.tcst, " + "-job summary -line_type TCMPR -column " + "'ABS(AMAX_WIND-BMAX_WIND)' " + "-out_stat out_stat/stat_summary.tcst")}, + ['dump_row', 'out_stat'], + ('jobs = ["-job summary -line_type TCMPR -column \'ABS(AMAX_WIND-BMAX_WIND)\' -dump_row dump_row/summary.tcst",' + '"-job summary -line_type TCMPR -column \'ABS(AMAX_WIND-BMAX_WIND)\' -out_stat out_stat/stat_summary.tcst"];')), + + # 1: dump_row file + ({'TC_STAT_JOB_ARGS': ("-job summary -line_type TCMPR -column " + "'ABS(AMAX_WIND-BMAX_WIND)' " + "-dump_row dump_row/summary.tcst")}, ['dump_row'], 'jobs = ["-job summary -line_type TCMPR -column \'ABS(AMAX_WIND-BMAX_WIND)\' -dump_row dump_row/summary.tcst"];'), + # 2: out_stat file + ({'TC_STAT_JOB_ARGS': ("-job summary -line_type TCMPR -column " + "'ABS(AMAX_WIND-BMAX_WIND)' " + "-out_stat out_stat/stat_summary.tcst")}, + ['out_stat'], + 'jobs = ["-job summary -line_type TCMPR -column \'ABS(AMAX_WIND-BMAX_WIND)\' -out_stat out_stat/stat_summary.tcst"];'), + ] +) +@pytest.mark.wrapper +def test_tc_stat_handle_jobs(metplus_config, config_overrides, expected_dirs, + expected_job_string): + config = get_config(metplus_config) + + # turn off "do not run" setting so directories are created + config.set('config', 'DO_NOT_RUN_EXE', False) + + # set config variable overrides + for key, value in config_overrides.items(): + config.set('config', key, value) + + # initialize wrapper and ensure it was initialized properly + wrapper = TCStatWrapper(config) + assert wrapper.isOK + + # ensure job string is formatted as expected + actual_job_string = wrapper.handle_jobs(time_info={}) + assert actual_job_string == expected_job_string + assert wrapper.env_var_dict['METPLUS_JOBS'] == expected_job_string + + # ensure output directories are created properly + for expected in expected_dirs: + expected_dir = os.path.join(wrapper.c_dict['OUTPUT_DIR'], expected) + assert os.path.exists(expected_dir) and os.path.isdir(expected_dir) + @pytest.mark.parametrize( 'config_overrides, env_var_values', [ # 0: no config overrides that set env vars From b1ee83699eb15fbdc1b91a0117e02332bd9241f3 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Tue, 11 Jul 2023 12:28:57 -0600 Subject: [PATCH 5/5] refactor and document new function --- metplus/wrappers/tc_stat_wrapper.py | 36 +++++++++++++++++++---------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/metplus/wrappers/tc_stat_wrapper.py b/metplus/wrappers/tc_stat_wrapper.py index 638420ba08..9c7b6722b4 100755 --- a/metplus/wrappers/tc_stat_wrapper.py +++ b/metplus/wrappers/tc_stat_wrapper.py @@ -366,9 +366,8 @@ def handle_jobs(self, time_info): formatted_jobs.append(subbed_job.strip()) # create parent directory of output file - for out in ('-dump_row', '-out_stat'): - if not self._create_job_out_dirs(out, subbed_job, time_info): - return None + if not self._create_job_out_dirs(subbed_job, time_info): + return None job_list_string = '","'.join(formatted_jobs) job_list_string = f'jobs = ["{job_list_string}"];' @@ -378,17 +377,30 @@ def handle_jobs(self, time_info): return job_list_string - def _create_job_out_dirs(self, out_type, job_args, time_info): + def _create_job_out_dirs(self, job_args, time_info): + """!Create output directories for output files specified by job args + like -dump_row and -out_stat to prevent the command from failing. + + @param job_args list of job arguments to parse + @param time_info time dictionary used to fill in filename + template tags if used + @returns False if something went wrong trying to create directories or + True if everything went smoothly. + """ split_job = job_args.split(' ') - # return True if job arg that writes a file is not found in job args - if out_type not in split_job: - return True + for out_type in ('-dump_row', '-out_stat'): + # continue if job arg that writes a file is not found in job args + if out_type not in split_job: + continue + + # if job arg is found, create parent directory of output file + index = split_job.index(out_type) + 1 + filepath = split_job[index] + self.c_dict['OUTPUT_TEMPLATE'] = filepath + if not self.find_and_check_output_file(time_info): + return False - # if job arg is found, create parent directory of output file - index = split_job.index(out_type) + 1 - filepath = split_job[index] - self.c_dict['OUTPUT_TEMPLATE'] = filepath - return self.find_and_check_output_file(time_info) + return True def handle_out_file(self, time_info): """! If output template is set,