From 8e984f46a9ba867c8d89ea1f04f9a827be1aeb49 Mon Sep 17 00:00:00 2001 From: Rodrigo V Honorato Date: Fri, 30 Aug 2024 10:38:08 +0200 Subject: [PATCH 1/8] refactor `validate_parameters_are_not_incompatible` --- src/haddock/gear/prepare_run.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/haddock/gear/prepare_run.py b/src/haddock/gear/prepare_run.py index 025c743da..2e82d9d22 100644 --- a/src/haddock/gear/prepare_run.py +++ b/src/haddock/gear/prepare_run.py @@ -962,9 +962,15 @@ def validate_parameters_are_not_incompatible(params: ParamMap) -> None: # Check if the limiting parameter is present in the parameters if limiting_param in params: # Check each incompatibility for the limiting parameter + incompatible_value_of_limiting_param = incompatibilities["value"] for incompatible_param, incompatible_value in incompatibilities.items(): + if incompatible_param == "value": + continue # Check if the incompatible parameter is present and has the incompatible value - if params.get(incompatible_param) == incompatible_value: + if ( + params.get(incompatible_param) == incompatible_value + and params[limiting_param] == incompatible_value_of_limiting_param + ): raise ValueError( f"Parameter `{limiting_param}` is incompatible with `{incompatible_param}={incompatible_value}`." ) From 7db0d9d150956e7bd243c115bfef2cfe92756347 Mon Sep 17 00:00:00 2001 From: Rodrigo V Honorato Date: Fri, 30 Aug 2024 10:38:19 +0200 Subject: [PATCH 2/8] add incompatible value --- src/haddock/modules/defaults.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/src/haddock/modules/defaults.yaml b/src/haddock/modules/defaults.yaml index 38f0ba99e..424dd192f 100644 --- a/src/haddock/modules/defaults.yaml +++ b/src/haddock/modules/defaults.yaml @@ -149,4 +149,5 @@ less_io: group: "execution" explevel: easy incompatible: + value: true mode: batch From f8dea55f632b1ac0cd3655c7660216e16fadfa63 Mon Sep 17 00:00:00 2001 From: Rodrigo V Honorato Date: Fri, 30 Aug 2024 10:38:26 +0200 Subject: [PATCH 3/8] update test --- tests/test_gear_prepare_run.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/test_gear_prepare_run.py b/tests/test_gear_prepare_run.py index f2ed4c0a6..44baf9658 100644 --- a/tests/test_gear_prepare_run.py +++ b/tests/test_gear_prepare_run.py @@ -392,19 +392,26 @@ def test_param_value_error(defaultparams, key, value): def test_validate_parameters_are_not_incompatible(mocker): mocker.patch( "haddock.gear.prepare_run.incompatible_defaults_params", - {"limiting_parameter": {"incompatible_parameter": "incompatible_value"}}, + { + "limiting_parameter": { + "value": True, + "incompatible_parameter": "incompatible_value", + } + }, ) + # Test 1 successfully fail params = { - "limiting_parameter": "", + "limiting_parameter": True, "incompatible_parameter": "incompatible_value", } with pytest.raises(ValueError): validate_parameters_are_not_incompatible(params) + # Test 2 - successfully pass params = { - "limiting_parameter": "limiting_value", + "limiting_parameter": False, "ok_parameter": "ok_value", } From 9e6bf4184d591fb583f3ec3e98ba984a2affbbf1 Mon Sep 17 00:00:00 2001 From: VGPReys Date: Mon, 2 Sep 2024 12:52:03 +0200 Subject: [PATCH 4/8] Proposing an other way to handle incompatibilities. - more flexible - can be used in other modules --- src/haddock/gear/prepare_run.py | 32 +++++++++++++++++-------------- src/haddock/modules/defaults.yaml | 4 ++-- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/haddock/gear/prepare_run.py b/src/haddock/gear/prepare_run.py index 2e82d9d22..5a1839a91 100644 --- a/src/haddock/gear/prepare_run.py +++ b/src/haddock/gear/prepare_run.py @@ -271,7 +271,11 @@ def setup_run( reference_parameters=ALL_POSSIBLE_GENERAL_PARAMETERS, ) - validate_parameters_are_not_incompatible(general_params) + # Validate there is no incompatible parameters in global parameters + validate_parameters_are_not_incompatible( + general_params, + incompatible_defaults_params, + ) # --extend-run configs do not define the run directory # in the config file. So we take it from the argument. @@ -944,7 +948,10 @@ def validate_module_names_are_not_misspelled(params: ParamMap) -> None: return -def validate_parameters_are_not_incompatible(params: ParamMap) -> None: +def validate_parameters_are_not_incompatible( + params: ParamMap, + incompatible_params: ParamMap, + ) -> None: """ Validate parameters are not incompatible. @@ -956,24 +963,21 @@ def validate_parameters_are_not_incompatible(params: ParamMap) -> None: Raises ------ ValueError - If any parameter in `params` is incompatible with another parameter as defined by `incompatible_params`. + If any parameter in `params` is incompatible with another parameter + as defined by `incompatible_params`. """ - for limiting_param, incompatibilities in incompatible_defaults_params.items(): + for limiting_param, incompatibilities in incompatible_params.items(): # Check if the limiting parameter is present in the parameters if limiting_param in params: # Check each incompatibility for the limiting parameter - incompatible_value_of_limiting_param = incompatibilities["value"] - for incompatible_param, incompatible_value in incompatibilities.items(): - if incompatible_param == "value": - continue + active_incompatibilities = incompatibilities[params[limiting_param]] + for incompatible_param, incompatible_value in active_incompatibilities.items(): # Check if the incompatible parameter is present and has the incompatible value - if ( - params.get(incompatible_param) == incompatible_value - and params[limiting_param] == incompatible_value_of_limiting_param - ): + if params.get(incompatible_param) == incompatible_value: raise ValueError( - f"Parameter `{limiting_param}` is incompatible with `{incompatible_param}={incompatible_value}`." - ) + f"Parameter `{limiting_param}` is incompatible with " + f"`{incompatible_param}={incompatible_value}`." + ) def validate_parameters_are_not_misspelled( diff --git a/src/haddock/modules/defaults.yaml b/src/haddock/modules/defaults.yaml index 424dd192f..a25966ec9 100644 --- a/src/haddock/modules/defaults.yaml +++ b/src/haddock/modules/defaults.yaml @@ -149,5 +149,5 @@ less_io: group: "execution" explevel: easy incompatible: - value: true - mode: batch + true: + mode: batch From ac6c4b5663e3254ea890f2a7b0aecfb44d9a3d0b Mon Sep 17 00:00:00 2001 From: VGPReys Date: Mon, 2 Sep 2024 13:11:43 +0200 Subject: [PATCH 5/8] add comment --- src/haddock/gear/prepare_run.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/haddock/gear/prepare_run.py b/src/haddock/gear/prepare_run.py index 5a1839a91..7881d6677 100644 --- a/src/haddock/gear/prepare_run.py +++ b/src/haddock/gear/prepare_run.py @@ -969,8 +969,9 @@ def validate_parameters_are_not_incompatible( for limiting_param, incompatibilities in incompatible_params.items(): # Check if the limiting parameter is present in the parameters if limiting_param in params: - # Check each incompatibility for the limiting parameter + # Point incompatibilities for the value of the limiting parameter active_incompatibilities = incompatibilities[params[limiting_param]] + # Check each incompatibility for the limiting parameter for incompatible_param, incompatible_value in active_incompatibilities.items(): # Check if the incompatible parameter is present and has the incompatible value if params.get(incompatible_param) == incompatible_value: From 758178871ca42d601f2a1f824d4bf36cb390ab82 Mon Sep 17 00:00:00 2001 From: VGPReys Date: Fri, 13 Sep 2024 09:33:56 +0200 Subject: [PATCH 6/8] fix code and update tests --- src/haddock/gear/prepare_run.py | 2 ++ tests/test_gear_prepare_run.py | 27 +++++++++++++++------------ 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/haddock/gear/prepare_run.py b/src/haddock/gear/prepare_run.py index 221923dc1..2099b671e 100644 --- a/src/haddock/gear/prepare_run.py +++ b/src/haddock/gear/prepare_run.py @@ -977,6 +977,8 @@ def validate_parameters_are_not_incompatible( # Check if the limiting parameter is present in the parameters if limiting_param in params: # Point incompatibilities for the value of the limiting parameter + if params[limiting_param] not in incompatibilities.keys(): + continue active_incompatibilities = incompatibilities[params[limiting_param]] # Check each incompatibility for the limiting parameter for incompatible_param, incompatible_value in active_incompatibilities.items(): diff --git a/tests/test_gear_prepare_run.py b/tests/test_gear_prepare_run.py index 44baf9658..613ccd3f8 100644 --- a/tests/test_gear_prepare_run.py +++ b/tests/test_gear_prepare_run.py @@ -389,17 +389,18 @@ def test_param_value_error(defaultparams, key, value): validate_value(defaultparams, key, value) -def test_validate_parameters_are_not_incompatible(mocker): - mocker.patch( - "haddock.gear.prepare_run.incompatible_defaults_params", - { - "limiting_parameter": { - "value": True, +def test_validate_parameters_are_not_incompatible(): + """Test parameter incompatibilities.""" + # Define an incompatible parameter + # when limiting_parameter has value `True`, + # `incompatible_parameter` cannot adopt `incompatible_value` + incompatible_params = { + "limiting_parameter": { + True: { "incompatible_parameter": "incompatible_value", } - }, - ) - + } + } # Test 1 successfully fail params = { "limiting_parameter": True, @@ -407,12 +408,14 @@ def test_validate_parameters_are_not_incompatible(mocker): } with pytest.raises(ValueError): - validate_parameters_are_not_incompatible(params) + validate_parameters_are_not_incompatible(params, incompatible_params) # Test 2 - successfully pass params = { "limiting_parameter": False, "ok_parameter": "ok_value", } - - assert validate_parameters_are_not_incompatible(params) is None + no_return = validate_parameters_are_not_incompatible( + params, incompatible_params + ) + assert no_return is None From c5c8b6239c0c6593c225e89762ffc0cb65294f6c Mon Sep 17 00:00:00 2001 From: VGPReys Date: Mon, 23 Sep 2024 10:28:14 +0200 Subject: [PATCH 7/8] fix merge --- tests/test_gear_prepare_run.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/test_gear_prepare_run.py b/tests/test_gear_prepare_run.py index e07f20e0d..ea2cea787 100644 --- a/tests/test_gear_prepare_run.py +++ b/tests/test_gear_prepare_run.py @@ -413,6 +413,10 @@ def test_param_value_error(defaultparams, key, value): def test_validate_parameters_are_not_incompatible(): """Test parameter incompatibilities.""" + params = { + "limiting_parameter": True, + "incompatible_parameter": "incompatible_value", + } # Define an incompatible parameter # when limiting_parameter has value `True`, # `incompatible_parameter` cannot adopt `incompatible_value` @@ -425,13 +429,16 @@ def test_validate_parameters_are_not_incompatible(): } # Test 1 successfully fail with pytest.raises(ValueError): - validate_parameters_are_not_incompatible(params, incompatible_params) + validate_parameters_are_not_incompatible( + params, + incompatible_params, + ) # Test 2 - successfully pass incompatible_params = { "limiting_parameter": { False: { - "ok_parameter": "ok_value", + "incompatible_parameter": "incompatible_value", } } } From db74e27eb2042d7d0f7a27acd8bff06c5f0ce9e2 Mon Sep 17 00:00:00 2001 From: VGPReys Date: Thu, 26 Sep 2024 13:33:32 +0200 Subject: [PATCH 8/8] extend incompatibility approach to all modules --- src/haddock/gear/prepare_run.py | 51 +++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 8 deletions(-) diff --git a/src/haddock/gear/prepare_run.py b/src/haddock/gear/prepare_run.py index 37f81dc31..2d6ef7144 100644 --- a/src/haddock/gear/prepare_run.py +++ b/src/haddock/gear/prepare_run.py @@ -64,7 +64,10 @@ v_rundir, validate_defaults_yaml, ) -from haddock.gear.yaml2cfg import read_from_yaml_config +from haddock.gear.yaml2cfg import ( + read_from_yaml_config, + find_incompatible_parameters, + ) from haddock.gear.zerofill import zero_fill from haddock.libs.libfunc import not_none from haddock.libs.libio import make_writeable_recursive @@ -179,16 +182,33 @@ def _read_defaults(module_name, default_only=True): default.yaml file of the module. """ module_name_ = get_module_name(module_name) - pdef = Path( + pdef = gen_defaults_module_param_path(module_name_) + validate_defaults_yaml(pdef) + mod_default_config = read_from_yaml_config(pdef, default_only=default_only) + return mod_default_config + + +def gen_defaults_module_param_path(module_name_: str) -> Path: + """Build path to default parameters of a module. + + Parameters + ---------- + module_name_ : str + Name of the module + + Returns + ------- + Path + Path to the module YAML defaults parameter. + """ + params_defaults_path = Path( haddock3_source_path, "modules", modules_category[module_name_], module_name_, "defaults.yaml", ).resolve() - validate_defaults_yaml(pdef) - mod_default_config = read_from_yaml_config(pdef, default_only=default_only) - return mod_default_config + return params_defaults_path def setup_run( @@ -535,8 +555,7 @@ def validate_modules_names(params: Iterable[str]) -> None: @with_config_error def validate_modules_params(modules_params: ParamMap, max_mols: int) -> None: - """ - Validate individual parameters for each module. + """Validate individual parameters for each module. Raises ------ @@ -550,6 +569,21 @@ def validate_modules_params(modules_params: ParamMap, max_mols: int) -> None: if not defaults: continue + # Check for parameter incompatibilities + module_incompatibilities = find_incompatible_parameters( + gen_defaults_module_param_path(module_name) + ) + try: + validate_parameters_are_not_incompatible( + args, + module_incompatibilities, + ) + except ValueError as e: + raise ConfigurationError( + f"An issue was discovered in module [{module_name}]: " + f"{e.args[0]}" + ) + if module_name in modules_using_resdic: confirm_resdic_chainid_length(args) @@ -1093,7 +1127,8 @@ def validate_parameters_are_not_incompatible( # Check if the incompatible parameter is present and has the incompatible value if params.get(incompatible_param) == incompatible_value: raise ValueError( - f"Parameter `{limiting_param}` is incompatible with " + f"Parameter `{limiting_param}` with value " + f"`{params[limiting_param]}` is incompatible with " f"`{incompatible_param}={incompatible_value}`." )