From 4d5eac3468cd64ae81480c1d10a935ec1d186026 Mon Sep 17 00:00:00 2001 From: Rajendra Adhikari Date: Mon, 5 Aug 2019 16:04:32 -0600 Subject: [PATCH 1/4] validate options --- buildstockbatch/base.py | 97 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 88 insertions(+), 9 deletions(-) diff --git a/buildstockbatch/base.py b/buildstockbatch/base.py index f3b06ba4..6ce2530f 100644 --- a/buildstockbatch/base.py +++ b/buildstockbatch/base.py @@ -23,6 +23,8 @@ import yaml import yamale import zipfile +import csv +import difflib from buildstockbatch.__version__ import __schema_version__ from .workflow_generator import ResidentialDefaultWorkflowGenerator, CommercialDefaultWorkflowGenerator @@ -282,21 +284,27 @@ def cleanup_sim_dir(sim_dir): def validate_project(project_file): assert(BuildStockBatchBase.validate_project_schema(project_file)) assert(BuildStockBatchBase.validate_xor_schema_keys(project_file)) + assert(BuildStockBatchBase.validate_options_lookup(project_file)) logger.info('Base Validation Successful') return True @staticmethod - def validate_project_schema(project_file): + def get_project_configuration(project_file): try: with open(project_file) as f: cfg = yaml.load(f, Loader=yaml.SafeLoader) except FileNotFoundError as err: - print(f'Failed to load input yaml for validation') + logger.error(f'Failed to load input yaml for validation') raise err + return cfg + + @staticmethod + def validate_project_schema(project_file): + cfg = BuildStockBatchBase.get_project_configuration(project_file) schema_version = cfg.get('schema_version', __schema_version__) version_schema = os.path.join(os.path.dirname(__file__), 'schemas', f'v{schema_version}.yaml') if not os.path.isfile(version_schema): - print(f'Could not find validation schema for YAML version {schema_version}') + logger.error(f'Could not find validation schema for YAML version {schema_version}') raise FileNotFoundError(version_schema) schema = yamale.make_schema(version_schema) data = yamale.make_data(project_file) @@ -304,12 +312,7 @@ def validate_project_schema(project_file): @staticmethod def validate_xor_schema_keys(project_file): - try: - with open(project_file) as f: - cfg = yaml.load(f, Loader=yaml.SafeLoader) - except FileNotFoundError as err: - print(f'Failed to load input yaml for validation') - raise err + cfg = BuildStockBatchBase.get_project_configuration(project_file) major, minor = cfg.get('version', __schema_version__).split('.') if int(major) >= 0: if int(minor) >= 0: @@ -321,6 +324,82 @@ def validate_xor_schema_keys(project_file): raise ValueError('Both/neither n_datapoints and buildstock_csv found in yaml baseline key') return True + @staticmethod + def validate_options_lookup(project_file): + cfg = BuildStockBatchBase.get_project_configuration(project_file) + param_option_dict = {} + options_lookup_path = f'{cfg["buildstock_directory"]}/resources/options_lookup.tsv' + try: + with open(options_lookup_path, 'r') as f: + options = csv.DictReader(f, delimiter='\t') + for row in options: + if row['Parameter Name'] not in param_option_dict: + param_option_dict[row['Parameter Name']] = set() + param_option_dict[row['Parameter Name']].add(row['Option Name']) + except FileNotFoundError as err: + logger.error(f"Options lookup file not found at: '{options_lookup_path}'") + raise err + + def get_errors(option_str): + if '|' not in option_str: + return f"* No option name delimiter '|' in '{option_str}' \n" + try: + parameter_name, option_name = option_str.split('|') + except ValueError: + return f"* Option specification '{option_str}' has too many '|'. \n" + + if parameter_name not in param_option_dict: + error_str = f"* Parameter name '{parameter_name}' does not exist in options_lookup. \n" + close_match = difflib.get_close_matches(parameter_name, param_option_dict.keys(),1) + if close_match: + error_str += f"Maybe you meant to type '{close_match[0]}'. \n" + return error_str + + if not option_name or option_name not in param_option_dict[parameter_name]: + error_str = f"* Option name '{option_name}' does not exist in options_lookup " \ + f"for paramter '{parameter_name}'. \n" + close_match = difflib.get_close_matches(option_name, list(param_option_dict[parameter_name]), 1) + if close_match: + error_str += f"Maybe you meant to type '{close_match[0]}'. \n" + return error_str + + return '' + + def get_all_str(inp): + if not inp: + return [] + if type(inp) == str: + return [inp] + elif type(inp) == list: + return sum([get_all_str(i) for i in inp], []) + elif type(inp) == dict: + return sum([get_all_str(i) for i in inp.values()], []) + + error_message = '' + option_strs = [] + if 'upgrades' in cfg: + for upgrade in cfg['upgrades']: + for option in upgrade['options']: + option_strs.append(option['option']) + if 'apply_logic' in option: + option_strs += get_all_str(option['apply_logic']) + + if 'package_apply_logic' in upgrade: + option_strs += get_all_str(upgrade['package_apply_logic']) + + if 'downselect' in cfg: + option_strs += get_all_str(cfg['downselect']['logic']) + print(f"resample? {cfg['downselect']['resample']}") + + for option_str in option_strs: + error_message += get_errors(option_str) + + if not error_message: + return True + else: + logger.error("Option/parameter name(s) is(are) invalid. \n" + error_message) + return False + def get_dask_client(self): return Client() From 5b60eef674140d09d47f1eff4f2aa938236748c7 Mon Sep 17 00:00:00 2001 From: Rajendra Adhikari Date: Mon, 5 Aug 2019 16:07:39 -0600 Subject: [PATCH 2/4] Scheme update to include downselect --- buildstockbatch/schemas/v0.1.yaml | 4 ++++ buildstockbatch/test/test_inputs/complete-schema.yml | 3 +++ 2 files changed, 7 insertions(+) diff --git a/buildstockbatch/schemas/v0.1.yaml b/buildstockbatch/schemas/v0.1.yaml index 18885b10..10dae077 100644 --- a/buildstockbatch/schemas/v0.1.yaml +++ b/buildstockbatch/schemas/v0.1.yaml @@ -9,6 +9,7 @@ sys_image_dir: str(required=False) baseline: include('sim-spec', required=True) timeseries_csv_export: map(required=False) upgrades: list(include('upgrade-spec'), required=False) +downselect: include('downselect-spec',required=False) postprocessing: include('postprocessing-spec', required=False) residential_simulation_controls: include('residential-simulation-spec', required=False) schema_version: num(required=False) @@ -61,6 +62,9 @@ option-spec: cost-spec: value: num(required=True) multiplier: str(required=False) +downselect-spec: + resample: bool(required=False) + logic: any(map(required=True),list(required=True),str(required=True)) postprocessing-spec: aws: include('aws-postprocessing-spec', required=False) aggregate_timeseries: bool(required=False) diff --git a/buildstockbatch/test/test_inputs/complete-schema.yml b/buildstockbatch/test/test_inputs/complete-schema.yml index f773cb67..1cde6de5 100644 --- a/buildstockbatch/test/test_inputs/complete-schema.yml +++ b/buildstockbatch/test/test_inputs/complete-schema.yml @@ -69,6 +69,9 @@ upgrades: timeseries_csv_export: reporting_frequency: Hourly include_enduse_subcategories: true +downselect: + resample: False + logic: HVAC System Heating Natural Gas|Gas Furnace, 60% AFUE eagle: account: enduse n_jobs: 100 From cf5d54fec15a29e7a97dacd90b825547357ff6ce Mon Sep 17 00:00:00 2001 From: Rajendra Adhikari Date: Tue, 6 Aug 2019 16:00:10 -0600 Subject: [PATCH 3/4] Tests for options validation --- buildstockbatch/base.py | 12 +++-- buildstockbatch/test/test_eagle.py | 6 ++- .../enforce-validate-options-bad.yml | 23 ++++++++++ .../enforce-validate-options-good.yml | 23 ++++++++++ .../resources/options_lookup.tsv | 26 +++++++++++ buildstockbatch/test/test_validation.py | 46 +++++++++++++++---- 6 files changed, 121 insertions(+), 15 deletions(-) create mode 100644 buildstockbatch/test/test_inputs/enforce-validate-options-bad.yml create mode 100644 buildstockbatch/test/test_inputs/enforce-validate-options-good.yml create mode 100644 buildstockbatch/test/test_inputs/test_openstudio_buildstock/resources/options_lookup.tsv diff --git a/buildstockbatch/base.py b/buildstockbatch/base.py index 6ce2530f..706f84d4 100644 --- a/buildstockbatch/base.py +++ b/buildstockbatch/base.py @@ -328,7 +328,9 @@ def validate_xor_schema_keys(project_file): def validate_options_lookup(project_file): cfg = BuildStockBatchBase.get_project_configuration(project_file) param_option_dict = {} - options_lookup_path = f'{cfg["buildstock_directory"]}/resources/options_lookup.tsv' + buildstock_dir = os.path.join(os.path.dirname(project_file), cfg["buildstock_directory"]) + options_lookup_path = f'{buildstock_dir}/resources/options_lookup.tsv' + try: with open(options_lookup_path, 'r') as f: options = csv.DictReader(f, delimiter='\t') @@ -350,7 +352,7 @@ def get_errors(option_str): if parameter_name not in param_option_dict: error_str = f"* Parameter name '{parameter_name}' does not exist in options_lookup. \n" - close_match = difflib.get_close_matches(parameter_name, param_option_dict.keys(),1) + close_match = difflib.get_close_matches(parameter_name, param_option_dict.keys(), 1) if close_match: error_str += f"Maybe you meant to type '{close_match[0]}'. \n" return error_str @@ -389,7 +391,6 @@ def get_all_str(inp): if 'downselect' in cfg: option_strs += get_all_str(cfg['downselect']['logic']) - print(f"resample? {cfg['downselect']['resample']}") for option_str in option_strs: error_message += get_errors(option_str) @@ -397,8 +398,9 @@ def get_all_str(inp): if not error_message: return True else: - logger.error("Option/parameter name(s) is(are) invalid. \n" + error_message) - return False + error_message = "Option/parameter name(s) is(are) invalid. \n" + error_message + logger.error(error_message) + raise ValueError(error_message) def get_dask_client(self): return Client() diff --git a/buildstockbatch/test/test_eagle.py b/buildstockbatch/test/test_eagle.py index 1e1da3a7..81ec4a36 100644 --- a/buildstockbatch/test/test_eagle.py +++ b/buildstockbatch/test/test_eagle.py @@ -7,12 +7,14 @@ from buildstockbatch.eagle import user_cli, EagleBatch +@patch('buildstockbatch.base.BuildStockBatchBase.validate_options_lookup') @patch('buildstockbatch.eagle.subprocess') -def test_user_cli(mock_subprocess, basic_residential_project_file): +def test_user_cli(mock_subprocess, mock_validate_options, basic_residential_project_file): + mock_validate_options.return_value = True + project_filename, results_dir = basic_residential_project_file() shutil.rmtree(results_dir) os.environ['CONDA_PREFIX'] = 'something' - argv = [project_filename] user_cli(argv) mock_subprocess.run.assert_called_once() diff --git a/buildstockbatch/test/test_inputs/enforce-validate-options-bad.yml b/buildstockbatch/test/test_inputs/enforce-validate-options-bad.yml new file mode 100644 index 00000000..6689a4b2 --- /dev/null +++ b/buildstockbatch/test/test_inputs/enforce-validate-options-bad.yml @@ -0,0 +1,23 @@ +buildstock_directory: test_openstudio_buildstock +project_directory: project_singlefamilydetached +baseline: + n_datapoints: 30 + n_buildings_represented: 81221016 +upgrades: + - upgrade_name: good upgrade + options: + - option: Vintage|Extra Argument + apply_logic: + - or: + - Insulation Slab|Invalid Option + - Insulation Slab|None + - not: Insulation Wall|Good Option + - and: + - Vintage|1960s + - Vintage| 1980s + - option: Insulation Finished Basement|Good Option + apply_logic: + - Insulation Unfinished Basement|Extra Argument +downselect: + resample: False + logic: Invalid Parameter|2000s diff --git a/buildstockbatch/test/test_inputs/enforce-validate-options-good.yml b/buildstockbatch/test/test_inputs/enforce-validate-options-good.yml new file mode 100644 index 00000000..32abb83c --- /dev/null +++ b/buildstockbatch/test/test_inputs/enforce-validate-options-good.yml @@ -0,0 +1,23 @@ +buildstock_directory: test_openstudio_buildstock +project_directory: project_singlefamilydetached +baseline: + n_datapoints: 30 + n_buildings_represented: 81221016 +upgrades: + - upgrade_name: good upgrade + options: + - option: Vintage|<1940 + apply_logic: + - or: + - Insulation Slab|Good Option + - Insulation Slab|None + - not: Insulation Wall|Good Option + - and: + - Vintage|1960s + - Vintage|1980s + - option: Insulation Finished Basement|Good Option + apply_logic: + - Insulation Unfinished Basement|Extra Argument +downselect: + resample: False + logic: Vintage|2000s diff --git a/buildstockbatch/test/test_inputs/test_openstudio_buildstock/resources/options_lookup.tsv b/buildstockbatch/test/test_inputs/test_openstudio_buildstock/resources/options_lookup.tsv new file mode 100644 index 00000000..0e001a78 --- /dev/null +++ b/buildstockbatch/test/test_inputs/test_openstudio_buildstock/resources/options_lookup.tsv @@ -0,0 +1,26 @@ +Parameter Name Option Name Measure Dir Measure Arg 1 Measure Arg 2 ... +Location AL_Birmingham.Muni.AP.722280 +Location AL_Huntsville.Intl.AP-Jones.Field.723230 +Location AL_Mobile-Rgnl.AP.722230 +Vintage <1940 +Vintage 1940s +Vintage <1950 +Vintage 1950s +Vintage 1960s +Vintage 1970s +Vintage 1980s +Vintage 1990s +Vintage 2000s +Vintage 2010s +Insulation Slab None +Insulation Slab Good Option ResidentialConstructionsSlab perimeter_r=0 perimeter_width=0 whole_r=0 gap_r=0 exterior_r=0 exterior_depth=0 +Insulation Slab Missing Argument ResidentialConstructionsSlab perimeter_r=0 perimeter_width=0 whole_r=10 gap_r=5 exterior_r=0 +Insulation Unfinished Basement None +Insulation Unfinished Basement Good Option ResidentialConstructionsUnfinishedBasement wall_ins_height=0 wall_cavity_r=0 wall_install_grade=1 wall_cavity_depth_in=0 wall_filled_cavity=true wall_framing_factor=0 wall_rigid_r=0 wall_drywall_thick_in=0.5 ceiling_cavity_r=0 ceiling_install_grade=1 ceiling_framing_factor=0.13 ceiling_joist_height_in=9.25 slab_whole_r=0 +Insulation Unfinished Basement Extra Argument ResidentialConstructionsUnfinishedBasement wall_ins_height=0 wall_cavity_r=0 wall_install_grade=1 wall_cavity_depth_in=0 wall_filled_cavity=true wall_framing_factor=0 wall_rigid_r=0 wall_drywall_thick_in=0.5 ceiling_cavity_r=13 ceiling_install_grade=1 ceiling_framing_factor=0.13 ceiling_joist_height_in=9.25 slab_whole_r=0 extra_arg=1 +Insulation Finished Basement None +Insulation Finished Basement Good Option ResidentialConstructionsFinishedBasement wall_ins_height=0 wall_cavity_r=0 wall_install_grade=1 wall_cavity_depth_in=0 wall_filled_cavity=true wall_framing_factor=0 wall_rigid_r=0 wall_drywall_thick_in=0.5 slab_whole_r=0 +Insulation Finished Basement Bad Value ResidentialConstructionsFinishedBasement wall_ins_height=4 wall_cavity_r=0 wall_install_grade=1 wall_cavity_depth_in=0 wall_filled_cavity=1.5 wall_framing_factor=0 wall_rigid_r=5 wall_drywall_thick_in=0.5 slab_whole_r=0 +Insulation Wall Good Option ResidentialConstructionsWallsWoodStud cavity_r=0 install_grade=1 cavity_depth_in=3.5 cavity_filled=false framing_factor=0.25 drywall_thick_in=0.5 osb_thick_in=0.5 rigid_r=0.0 "exterior_finish=Vinyl, Light" +Insulation Wall Missing Measure ResidentialConstructionsWallsWoodStud cavity_r=0 install_grade=1 cavity_depth_in=3.5 cavity_filled=false framing_factor=0.25 drywall_thick_in=0.5 osb_thick_in=0.5 rigid_r=0.0 "exterior_finish=Vinyl, Light" + ResidentialMissingMeasure diff --git a/buildstockbatch/test/test_validation.py b/buildstockbatch/test/test_validation.py index d88c2d7f..9e944513 100644 --- a/buildstockbatch/test/test_validation.py +++ b/buildstockbatch/test/test_validation.py @@ -17,6 +17,7 @@ from buildstockbatch.eagle import EagleBatch from buildstockbatch.localdocker import LocalDockerBatch from buildstockbatch.base import BuildStockBatchBase +from unittest.mock import patch here = os.path.dirname(os.path.abspath(__file__)) example_yml_dir = os.path.join(here, 'test_inputs') @@ -51,8 +52,10 @@ def test_minimal_schema_passes_validation(): os.path.join(example_yml_dir, 'missing-nested-required-schema.yml') ]) def test_missing_required_key_fails(project_file): - with pytest.raises(ValueError): - BuildStockBatchBase.validate_project_schema(project_file) + # patch the validate_options_lookup function to always return true for this case + with patch.object(BuildStockBatchBase, 'validate_options_lookup', lambda _: True): + with pytest.raises(ValueError): + BuildStockBatchBase.validate_project_schema(project_file) @pytest.mark.parametrize("project_file", [ @@ -61,8 +64,10 @@ def test_missing_required_key_fails(project_file): os.path.join(example_yml_dir, 'enforce-schema-xor.yml') ]) def test_xor_violations_fail(project_file): - with pytest.raises(ValueError): - BuildStockBatchBase.validate_xor_schema_keys(project_file) + # patch the validate_options_lookup function to always return true for this case + with patch.object(BuildStockBatchBase, 'validate_options_lookup', lambda _: True): + with pytest.raises(ValueError): + BuildStockBatchBase.validate_xor_schema_keys(project_file) @pytest.mark.parametrize("project_file,expected", [ @@ -75,8 +80,33 @@ def test_xor_violations_fail(project_file): (os.path.join(example_yml_dir, 'minimal-schema.yml'), True) ]) def test_validation_integration(project_file, expected): - if expected is not True: - with pytest.raises(expected): - BuildStockBatchBase.validate_project(project_file) + # patch the validate_options_lookup function to always return true for this case + with patch.object(BuildStockBatchBase, 'validate_options_lookup', lambda _: True): + if expected is not True: + with pytest.raises(expected): + BuildStockBatchBase.validate_project(project_file) + else: + assert(BuildStockBatchBase.validate_project(project_file)) + + +@pytest.mark.parametrize("project_file", [ + os.path.join(example_yml_dir, 'enforce-validate-options-good.yml'), +]) +def test_good_options_validation(project_file): + assert BuildStockBatchBase.validate_options_lookup(project_file) + + +@pytest.mark.parametrize("project_file", [ + os.path.join(example_yml_dir, 'enforce-validate-options-bad.yml'), +]) +def test_bad_options_validation(project_file): + try: + BuildStockBatchBase.validate_options_lookup(project_file) + except ValueError as er: + er = str(er) + assert "Extra Argument" in er + assert "Invalid Option" in er + assert "Invalid Parameter" in er + assert " 1980s" in er else: - assert(BuildStockBatchBase.validate_project(project_file)) + raise Exception("validate_options was supposed to raise ValueError for enforce-validate-options-bad.yml") From 6b89a3f7507f13011b751a7807558a646a83f592 Mon Sep 17 00:00:00 2001 From: Rajendra Adhikari Date: Wed, 7 Aug 2019 15:27:33 -0600 Subject: [PATCH 4/4] Added robustness and documentation. --- buildstockbatch/base.py | 91 +++++++++++++++---- .../enforce-validate-options-bad.yml | 19 +++- .../enforce-validate-options-good.yml | 3 +- .../enforce-validate-options-wrong-path.yml | 24 +++++ buildstockbatch/test/test_validation.py | 17 +++- 5 files changed, 131 insertions(+), 23 deletions(-) create mode 100644 buildstockbatch/test/test_inputs/enforce-validate-options-wrong-path.yml diff --git a/buildstockbatch/base.py b/buildstockbatch/base.py index 706f84d4..cd0f253e 100644 --- a/buildstockbatch/base.py +++ b/buildstockbatch/base.py @@ -326,11 +326,15 @@ def validate_xor_schema_keys(project_file): @staticmethod def validate_options_lookup(project_file): + """ + Validates that the parameter|options specified in the project yaml file is avaliable in the options_lookup.tsv + """ cfg = BuildStockBatchBase.get_project_configuration(project_file) param_option_dict = {} buildstock_dir = os.path.join(os.path.dirname(project_file), cfg["buildstock_directory"]) options_lookup_path = f'{buildstock_dir}/resources/options_lookup.tsv' + # fill in the param_option_dict with {'param1':['valid_option1','valid_option2' ...]} from options_lookup.tsv try: with open(options_lookup_path, 'r') as f: options = csv.DictReader(f, delimiter='\t') @@ -342,58 +346,107 @@ def validate_options_lookup(project_file): logger.error(f"Options lookup file not found at: '{options_lookup_path}'") raise err - def get_errors(option_str): - if '|' not in option_str: - return f"* No option name delimiter '|' in '{option_str}' \n" + def get_errors(source_str, option_str): + """ + Gives multiline descriptive error message if the option_str is invalid. Returns '' otherwise + :param source_str: the descriptive location where the option_str occurs in the yaml configuration. + :param option_str: the param|option string representing the option choice. Can be joined by either || or && + to form composite string. eg. param1|option1||param2|option2 + :return: returns empty string if the param|option is valid i.e. they are found in options_lookup.tsv + if not returns error message, close matches, and specifies where the error occurred (source_str) + """ + if '||' in option_str and '&&' in option_str: + return f"* Option specification '{option_str}' has both || and &&, which is not supported. " \ + f"{source_str}\n" + + if '||' in option_str or '&&' in option_str: + splitter = '||' if '||' in option_str else '&&' + errors = '' + broken_options = option_str.split(splitter) + if broken_options[-1] == '': + return f"* Option spec '{option_str}' has a trailing '{splitter}'. {source_str}\n" + for broken_option_str in broken_options: + new_source_str = source_str + f" in composite option '{option_str}'" + errors += get_errors(new_source_str, broken_option_str) + return errors + + if not option_str or '|' == option_str: + return f"* Option name empty. {source_str}\n" + try: parameter_name, option_name = option_str.split('|') except ValueError: - return f"* Option specification '{option_str}' has too many '|'. \n" + return f"* Option specification '{option_str}' has too many or too few '|' (exactly 1 required)." \ + f" {source_str}\n" if parameter_name not in param_option_dict: error_str = f"* Parameter name '{parameter_name}' does not exist in options_lookup. \n" close_match = difflib.get_close_matches(parameter_name, param_option_dict.keys(), 1) if close_match: error_str += f"Maybe you meant to type '{close_match[0]}'. \n" + error_str += f"{source_str}\n" return error_str if not option_name or option_name not in param_option_dict[parameter_name]: error_str = f"* Option name '{option_name}' does not exist in options_lookup " \ - f"for paramter '{parameter_name}'. \n" + f"for parameter '{parameter_name}'. \n" close_match = difflib.get_close_matches(option_name, list(param_option_dict[parameter_name]), 1) if close_match: error_str += f"Maybe you meant to type '{close_match[0]}'. \n" + error_str += f"{source_str}\n" return error_str return '' - def get_all_str(inp): + def get_all_option_str(source_str, inp): + """ + Returns a list of (source_str, option_str) tuple by recursively traversing the logic inp structure. + Check the get_errors function for more info about source_str and option_str + :param source_str: the descriptive location where the inp logic is found + :param inp: A nested apply_logic structure + :return: List of tuples of (source_str, option_str) where source_str is the location in inp where the + option_str is found. + """ if not inp: return [] if type(inp) == str: - return [inp] + return [(source_str, inp)] elif type(inp) == list: - return sum([get_all_str(i) for i in inp], []) + return sum([get_all_option_str(source_str + f", in entry {count}", entry) for count, entry + in enumerate(inp)], []) elif type(inp) == dict: - return sum([get_all_str(i) for i in inp.values()], []) + if len(inp) > 1: + raise ValueError(f"{source_str} the logic is malformed.") + source_str += f", in {list(inp.keys())[0]}" + return sum([get_all_option_str(source_str, i) for i in inp.values()], []) + + # store all of the option_str in the project file as a list of (source_str, option_str) tuple + source_option_str_list = [] - error_message = '' - option_strs = [] if 'upgrades' in cfg: - for upgrade in cfg['upgrades']: - for option in upgrade['options']: - option_strs.append(option['option']) + for upgrade_count, upgrade in enumerate(cfg['upgrades']): + upgrade_name = upgrade.get('upgrade_name', '') + f' (Upgrade Number: {upgrade_count})' + source_str_upgrade = f"In upgrade '{upgrade_name}'" + for option_count, option in enumerate(upgrade['options']): + option_name = option.get('option', '') + f' (Option Number: {option_count})' + source_str_option = source_str_upgrade + f", in option '{option_name}'" + source_option_str_list.append((source_str_option, option.get('option'))) if 'apply_logic' in option: - option_strs += get_all_str(option['apply_logic']) + source_str_logic = source_str_option + ", in apply_logic" + source_option_str_list += get_all_option_str(source_str_logic, option['apply_logic']) if 'package_apply_logic' in upgrade: - option_strs += get_all_str(upgrade['package_apply_logic']) + source_str_package = source_str_upgrade + ", in package_apply_logic" + source_option_str_list += get_all_option_str(source_str_package, upgrade['package_apply_logic']) if 'downselect' in cfg: - option_strs += get_all_str(cfg['downselect']['logic']) + source_str = f"In downselect" + source_option_str_list += get_all_option_str(source_str, cfg['downselect']['logic']) - for option_str in option_strs: - error_message += get_errors(option_str) + # Gather all the errors in the option_str, if any + error_message = '' + for source_str, option_str in source_option_str_list: + error_message += get_errors(source_str, option_str) if not error_message: return True diff --git a/buildstockbatch/test/test_inputs/enforce-validate-options-bad.yml b/buildstockbatch/test/test_inputs/enforce-validate-options-bad.yml index 6689a4b2..2a89d71b 100644 --- a/buildstockbatch/test/test_inputs/enforce-validate-options-bad.yml +++ b/buildstockbatch/test/test_inputs/enforce-validate-options-bad.yml @@ -4,20 +4,35 @@ baseline: n_datapoints: 30 n_buildings_represented: 81221016 upgrades: - - upgrade_name: good upgrade + - upgrade_name: bad upgrade options: - option: Vintage|Extra Argument apply_logic: - or: - Insulation Slab|Invalid Option - Insulation Slab|None - - not: Insulation Wall|Good Option + - not: Insulation Wall|Good Option|| - and: - Vintage|1960s - Vintage| 1980s - option: Insulation Finished Basement|Good Option apply_logic: - Insulation Unfinished Basement|Extra Argument + package_apply_logic: Vintage|1970s||Vintage|1941s + - options: + - option: | + apply_logic: + - or: + - Insulation Slat|Good Option + - Insulation Slab|None + - not: Insulation Wall|Good Option + - and: + - Vintage|1960s|Vintage|1960s + - Vintage|1980s + - option: Insulation Finished Basement|Good Option + apply_logic: + - Insulation Unfinished Basement|Extra Argument + package_apply_logic: Vintage|1960s||Vintage|1940s&&Vintage|1980s downselect: resample: False logic: Invalid Parameter|2000s diff --git a/buildstockbatch/test/test_inputs/enforce-validate-options-good.yml b/buildstockbatch/test/test_inputs/enforce-validate-options-good.yml index 32abb83c..135ff82e 100644 --- a/buildstockbatch/test/test_inputs/enforce-validate-options-good.yml +++ b/buildstockbatch/test/test_inputs/enforce-validate-options-good.yml @@ -13,11 +13,12 @@ upgrades: - Insulation Slab|None - not: Insulation Wall|Good Option - and: - - Vintage|1960s + - Vintage|1960s||Vintage|1960s - Vintage|1980s - option: Insulation Finished Basement|Good Option apply_logic: - Insulation Unfinished Basement|Extra Argument + package_apply_logic: Vintage|1960s||Vintage|1940s downselect: resample: False logic: Vintage|2000s diff --git a/buildstockbatch/test/test_inputs/enforce-validate-options-wrong-path.yml b/buildstockbatch/test/test_inputs/enforce-validate-options-wrong-path.yml new file mode 100644 index 00000000..37668350 --- /dev/null +++ b/buildstockbatch/test/test_inputs/enforce-validate-options-wrong-path.yml @@ -0,0 +1,24 @@ +buildstock_directory: test_openstudio_buildstock_wrong +project_directory: project_singlefamilydetached +baseline: + n_datapoints: 30 + n_buildings_represented: 81221016 +upgrades: + - upgrade_name: good upgrade + options: + - option: Vintage|<1940 + apply_logic: + - or: + - Insulation Slab|Good Option + - Insulation Slab|None + - not: Insulation Wall|Good Option + - and: + - Vintage|1960s||Vintage|1960s + - Vintage|1980s + - option: Insulation Finished Basement|Good Option + apply_logic: + - Insulation Unfinished Basement|Extra Argument + package_apply_logic: Vintage|1960s||Vintage|1940s +downselect: + resample: False + logic: Vintage|2000s diff --git a/buildstockbatch/test/test_validation.py b/buildstockbatch/test/test_validation.py index 9e944513..254b5da1 100644 --- a/buildstockbatch/test/test_validation.py +++ b/buildstockbatch/test/test_validation.py @@ -89,6 +89,14 @@ def test_validation_integration(project_file, expected): assert(BuildStockBatchBase.validate_project(project_file)) +@pytest.mark.parametrize("project_file", [ + os.path.join(example_yml_dir, 'enforce-validate-options-wrong-path.yml'), +]) +def test_bad_path_options_validation(project_file): + with pytest.raises(FileNotFoundError): + BuildStockBatchBase.validate_options_lookup(project_file) + + @pytest.mark.parametrize("project_file", [ os.path.join(example_yml_dir, 'enforce-validate-options-good.yml'), ]) @@ -106,7 +114,14 @@ def test_bad_options_validation(project_file): er = str(er) assert "Extra Argument" in er assert "Invalid Option" in er - assert "Invalid Parameter" in er + assert "Insulation Wall|Good Option||" in er assert " 1980s" in er + assert "Vintage|1941s" in er + assert "Option name empty" in er + assert "Insulation Slat" in er + assert "Vintage|1960s|Vintage|1960s" in er + assert "Vintage|1960s||Vintage|1940s&&Vintage|1980s" in er + assert "Invalid Parameter" in er + else: raise Exception("validate_options was supposed to raise ValueError for enforce-validate-options-bad.yml")