From 3b2f306bb6cd6401752e7a7706825693d86a1262 Mon Sep 17 00:00:00 2001 From: "Yngve S. Kristiansen" Date: Mon, 8 Jul 2024 13:27:41 +0200 Subject: [PATCH] Make ErtConfig fetch its own plugins --- src/ert/cli/main.py | 4 +-- src/ert/config/ert_config.py | 6 +++- src/ert/gui/main.py | 4 +-- src/ert/libres_facade.py | 6 ++-- .../workflows/disable_parameters.py | 4 +-- .../workflows/export_misfit_data.py | 2 +- .../workflows/export_runpath.py | 2 +- .../workflows/misfit_preprocessor.py | 4 +-- .../forward_model_steps.py | 2 +- src/ert/shared/plugins/plugin_manager.py | 9 +++--- tests/conftest.py | 7 ---- tests/integration_tests/test_cli.py | 23 +------------ .../test_storage_migration.py | 7 ++-- tests/unit_tests/config/test_forward_model.py | 32 ++++++++----------- tests/unit_tests/shared/share/test_shell.py | 4 +-- tests/unit_tests/test_run_path_creation.py | 6 ++-- 16 files changed, 43 insertions(+), 79 deletions(-) diff --git a/src/ert/cli/main.py b/src/ert/cli/main.py index 52bb625ee13..7554ce7e753 100644 --- a/src/ert/cli/main.py +++ b/src/ert/cli/main.py @@ -44,9 +44,7 @@ def run_cli(args: Namespace, plugin_manager: Optional[ErtPluginManager] = None) # the config file to be the base name of the original config args.config = os.path.basename(args.config) - ert_config = ErtConfig.with_plugins( - plugin_manager.forward_model_steps if plugin_manager else [] - ).from_file(args.config) + ert_config = ErtConfig.with_plugins().from_file(args.config) local_storage_set_ert_config(ert_config) diff --git a/src/ert/config/ert_config.py b/src/ert/config/ert_config.py index d162f2845c6..250f7c199eb 100644 --- a/src/ert/config/ert_config.py +++ b/src/ert/config/ert_config.py @@ -27,6 +27,7 @@ from typing_extensions import Self from ert.config.gen_data_config import GenDataConfig +from ert.shared.plugins import ErtPluginManager from ert.substitution_list import SubstitutionList from ._get_num_cpu import get_num_cpu_from_data_file @@ -126,8 +127,11 @@ def __post_init__(self) -> None: @staticmethod def with_plugins( - forward_model_step_classes: List[Type[ForwardModelStepPlugin]], + forward_model_step_classes: Optional[List[Type[ForwardModelStepPlugin]]] = None, ) -> Type["ErtConfig"]: + if forward_model_step_classes is None: + forward_model_step_classes = ErtPluginManager().forward_model_steps + preinstalled_fm_steps: Dict[str, ForwardModelStepPlugin] = {} for fm_step_subclass in forward_model_step_classes: fm_step = fm_step_subclass() diff --git a/src/ert/gui/main.py b/src/ert/gui/main.py index 4c3485e1816..ba6ed05e1f4 100755 --- a/src/ert/gui/main.py +++ b/src/ert/gui/main.py @@ -104,9 +104,7 @@ def _start_initial_gui_window( # the config file to be the base name of the original config args.config = os.path.basename(args.config) - ert_config = ErtConfig.with_plugins( - plugin_manager.forward_model_steps if plugin_manager else [] - ).from_file(args.config) + ert_config = ErtConfig.with_plugins().from_file(args.config) local_storage_set_ert_config(ert_config) except ConfigValidationError as error: diff --git a/src/ert/libres_facade.py b/src/ert/libres_facade.py index 6b069078d30..6ff16e2fd4d 100644 --- a/src/ert/libres_facade.py +++ b/src/ert/libres_facade.py @@ -278,10 +278,8 @@ def run_ertscript( # type: ignore def from_config_file( cls, config_file: str, read_only: bool = False ) -> "LibresFacade": - with ErtPluginContext() as ctx: + with ErtPluginContext(): return cls( - ErtConfig.with_plugins( - forward_model_step_classes=ctx.plugin_manager.forward_model_steps - ).from_file(config_file), + ErtConfig.with_plugins().from_file(config_file), read_only, ) diff --git a/src/ert/shared/hook_implementations/workflows/disable_parameters.py b/src/ert/shared/hook_implementations/workflows/disable_parameters.py index db7cd89fc73..c5ee7b4d1a4 100644 --- a/src/ert/shared/hook_implementations/workflows/disable_parameters.py +++ b/src/ert/shared/hook_implementations/workflows/disable_parameters.py @@ -1,7 +1,7 @@ from typing import Any, List -from ert import ErtScript -from ert.config import ConfigValidationError +from ert.config.ert_script import ErtScript +from ert.config.parsing.config_errors import ConfigValidationError class DisableParametersUpdate(ErtScript): diff --git a/src/ert/shared/hook_implementations/workflows/export_misfit_data.py b/src/ert/shared/hook_implementations/workflows/export_misfit_data.py index b8ecd2a0786..1e79e7647bf 100644 --- a/src/ert/shared/hook_implementations/workflows/export_misfit_data.py +++ b/src/ert/shared/hook_implementations/workflows/export_misfit_data.py @@ -4,7 +4,7 @@ import pandas as pd -from ert import ErtScript +from ert.config.ert_script import ErtScript from ert.exceptions import StorageError if TYPE_CHECKING: diff --git a/src/ert/shared/hook_implementations/workflows/export_runpath.py b/src/ert/shared/hook_implementations/workflows/export_runpath.py index 58c4b3125a0..178adefa37a 100644 --- a/src/ert/shared/hook_implementations/workflows/export_runpath.py +++ b/src/ert/shared/hook_implementations/workflows/export_runpath.py @@ -2,7 +2,7 @@ from typing import TYPE_CHECKING, Any, List, Tuple -from ert.config import ErtScript +from ert.config.ert_script import ErtScript from ert.runpaths import Runpaths from ert.validation import rangestring_to_mask diff --git a/src/ert/shared/hook_implementations/workflows/misfit_preprocessor.py b/src/ert/shared/hook_implementations/workflows/misfit_preprocessor.py index b780dea9e3b..17fb031f3c9 100644 --- a/src/ert/shared/hook_implementations/workflows/misfit_preprocessor.py +++ b/src/ert/shared/hook_implementations/workflows/misfit_preprocessor.py @@ -1,7 +1,7 @@ from typing import Any, List -from ert import ErtScript -from ert.config import ConfigValidationError +from ert.config.ert_script import ErtScript +from ert.config.parsing.config_errors import ConfigValidationError class MisfitPreprocessor(ErtScript): diff --git a/src/ert/shared/plugins/hook_specifications/forward_model_steps.py b/src/ert/shared/plugins/hook_specifications/forward_model_steps.py index 4763c0edaeb..99f36ab7e9b 100644 --- a/src/ert/shared/plugins/hook_specifications/forward_model_steps.py +++ b/src/ert/shared/plugins/hook_specifications/forward_model_steps.py @@ -2,10 +2,10 @@ from typing import TYPE_CHECKING, Dict, List, Optional, Type, no_type_check -from ert.config import ForwardModelStepPlugin from ert.shared.plugins.plugin_manager import hook_specification if TYPE_CHECKING: + from ert.config import ForwardModelStepPlugin from ert.shared.plugins.plugin_response import PluginResponse diff --git a/src/ert/shared/plugins/plugin_manager.py b/src/ert/shared/plugins/plugin_manager.py index c12371aabb6..ac723302f18 100644 --- a/src/ert/shared/plugins/plugin_manager.py +++ b/src/ert/shared/plugins/plugin_manager.py @@ -29,10 +29,6 @@ import pluggy -from ert.config.forward_model_step import ( - ForwardModelStepDocumentation, - ForwardModelStepPlugin, -) from ert.shared.plugins.workflow_config import WorkflowConfigs _PLUGIN_NAMESPACE = "ert" @@ -46,6 +42,11 @@ import ert.shared.plugins.hook_specifications # noqa if TYPE_CHECKING: + from ert.config.forward_model_step import ( + ForwardModelStepDocumentation, + ForwardModelStepPlugin, + ) + from .plugin_response import PluginMetadata, PluginResponse K = TypeVar("K") diff --git a/tests/conftest.py b/tests/conftest.py index 3d1a66e9911..adcbc9e44f5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -15,7 +15,6 @@ from qtpy.QtWidgets import QApplication from _ert.threading import set_signal_handler -from ert.shared.plugins import ErtPluginManager if sys.version_info >= (3, 9): from importlib.resources import files @@ -159,12 +158,6 @@ def snake_oil_case(setup_case): return setup_case("snake_oil", "snake_oil.ert") -@pytest.fixture() -def ErtConfigWithPlugins(): - pm = ErtPluginManager() - return ErtConfig.with_plugins(forward_model_step_classes=pm.forward_model_steps) - - @pytest.fixture() def minimum_case(use_tmpdir): with open("minimum_config", "w", encoding="utf-8") as fout: diff --git a/tests/integration_tests/test_cli.py b/tests/integration_tests/test_cli.py index c9d777ca4e7..2ad89463bfa 100644 --- a/tests/integration_tests/test_cli.py +++ b/tests/integration_tests/test_cli.py @@ -29,30 +29,9 @@ ITERATIVE_ENSEMBLE_SMOOTHER_MODE, TEST_RUN_MODE, ) -from ert.shared.plugins import ErtPluginManager from ert.storage import open_storage -from tests.unit_tests.all.plugins import dummy_plugins -from .run_cli import run_cli, run_cli_with_pm - - -def test_that_cli_runs_forward_model_from_plugin(tmp_path): - test_config_contents = dedent( - """ - NUM_REALIZATIONS 1 - FORWARD_MODEL DummyForwardModel - """ - ) - with open(tmp_path / "test.ert", "w", encoding="utf-8") as fh: - fh.write(test_config_contents) - - pm = ErtPluginManager(plugins=[dummy_plugins]) - run_cli_with_pm( - [TEST_RUN_MODE, "--disable-monitor", str(tmp_path / "test.ert")], pm - ) - assert os.path.exists( - tmp_path / "simulations" / "realization-0" / "iter-0" / "dummy.out" - ) +from .run_cli import run_cli @pytest.mark.filterwarnings("ignore::ert.config.ConfigWarning") diff --git a/tests/integration_tests/test_storage_migration.py b/tests/integration_tests/test_storage_migration.py index 0185153af09..5a1162decb0 100644 --- a/tests/integration_tests/test_storage_migration.py +++ b/tests/integration_tests/test_storage_migration.py @@ -5,6 +5,7 @@ import pytest from packaging import version +from ert.config import ErtConfig from ert.storage import open_storage from ert.storage.local_storage import local_storage_set_ert_config @@ -93,14 +94,13 @@ def test_that_storage_matches( snapshot, monkeypatch, ert_version, - ErtConfigWithPlugins, ): shutil.copytree( block_storage_path / f"all_data_types/storage-{ert_version}", tmp_path / "all_data_types" / f"storage-{ert_version}", ) monkeypatch.chdir(tmp_path / "all_data_types") - ert_config = ErtConfigWithPlugins.from_file("config.ert") + ert_config = ErtConfig.with_plugins().from_file("config.ert") local_storage_set_ert_config(ert_config) # To make sure all tests run against the same snapshot snapshot.snapshot_dir = snapshot.snapshot_dir.parent @@ -224,7 +224,6 @@ def test_that_storage_works_with_missing_parameters_and_responses( snapshot, monkeypatch, ert_version, - ErtConfigWithPlugins, ): storage_path = tmp_path / "all_data_types" / f"storage-{ert_version}" shutil.copytree( @@ -246,7 +245,7 @@ def test_that_storage_works_with_missing_parameters_and_responses( os.remove(real_dir / "GEN.nc") monkeypatch.chdir(tmp_path / "all_data_types") - ert_config = ErtConfigWithPlugins.from_file("config.ert") + ert_config = ErtConfig.with_plugins().from_file("config.ert") local_storage_set_ert_config(ert_config) # To make sure all tests run against the same snapshot snapshot.snapshot_dir = snapshot.snapshot_dir.parent diff --git a/tests/unit_tests/config/test_forward_model.py b/tests/unit_tests/config/test_forward_model.py index a47ad240ca1..8746d51fb83 100644 --- a/tests/unit_tests/config/test_forward_model.py +++ b/tests/unit_tests/config/test_forward_model.py @@ -39,7 +39,7 @@ def test_ert_config_throws_on_missing_forward_model_step( @pytest.mark.usefixtures("use_tmpdir") -def test_that_substitutions_can_be_done_in_job_names(ErtConfigWithPlugins): +def test_that_substitutions_can_be_done_in_job_names(): """ Regression test for a usage case involving setting ECL100 or ECL300 that was broken by changes to forward_model substitutions. @@ -55,14 +55,14 @@ def test_that_substitutions_can_be_done_in_job_names(ErtConfigWithPlugins): with open(test_config_file_name, "w", encoding="utf-8") as fh: fh.write(test_config_contents) - ert_config = ErtConfigWithPlugins.from_file(test_config_file_name) + ert_config = ErtConfig.with_plugins().from_file(test_config_file_name) assert len(ert_config.forward_model_steps) == 1 job = ert_config.forward_model_steps[0] assert job.name == "ECLIPSE100" @pytest.mark.usefixtures("use_tmpdir") -def test_parsing_forward_model_with_double_dash_is_possible(ErtConfigWithPlugins): +def test_parsing_forward_model_with_double_dash_is_possible(): """This is a regression test, making sure that we can put double dashes in strings. The use case is that a file name is utilized that contains two consecutive hyphens, which by the ert config parser used to be interpreted as a comment. In the new @@ -79,7 +79,7 @@ def test_parsing_forward_model_with_double_dash_is_possible(ErtConfigWithPlugins with open(test_config_file_name, "w", encoding="utf-8") as fh: fh.write(test_config_contents) - res_config = ErtConfigWithPlugins.from_file(test_config_file_name) + res_config = ErtConfig.with_plugins().from_file(test_config_file_name) assert res_config.model_config.jobname_format_string == "job_--hei" assert ( res_config.forward_model_steps[0].private_args[""] @@ -88,9 +88,7 @@ def test_parsing_forward_model_with_double_dash_is_possible(ErtConfigWithPlugins @pytest.mark.usefixtures("use_tmpdir") -def test_parsing_forward_model_with_quotes_does_not_introduce_spaces( - ErtConfigWithPlugins, -): +def test_parsing_forward_model_with_quotes_does_not_introduce_spaces(): """this is a regression test, making sure that we do not by mistake introduce spaces while parsing forward model lines that contain quotation marks @@ -110,7 +108,7 @@ def test_parsing_forward_model_with_quotes_does_not_introduce_spaces( with open(test_config_file_name, "w", encoding="utf-8") as fh: fh.write(test_config_contents) - ert_config = ErtConfigWithPlugins.from_file(test_config_file_name) + ert_config = ErtConfig.with_plugins().from_file(test_config_file_name) assert list(ert_config.forward_model_steps[0].private_args.values()) == [ "foo", "smt//bar/xx/t--s.s/yy/z/z/oo", @@ -118,7 +116,7 @@ def test_parsing_forward_model_with_quotes_does_not_introduce_spaces( @pytest.mark.usefixtures("use_tmpdir") -def test_that_comments_are_ignored(ErtConfigWithPlugins): +def test_that_comments_are_ignored(): """This is a regression test, making sure that we can put double dashes in strings. The use case is that a file name is utilized that contains two consecutive hyphens, which by the ert config parser used to be interpreted as a comment. In the new @@ -136,7 +134,7 @@ def test_that_comments_are_ignored(ErtConfigWithPlugins): with open(test_config_file_name, "w", encoding="utf-8") as fh: fh.write(test_config_contents) - res_config = ErtConfigWithPlugins.from_file(test_config_file_name) + res_config = ErtConfig.with_plugins().from_file(test_config_file_name) assert res_config.model_config.jobname_format_string == "job_--hei" assert ( res_config.forward_model_steps[0].private_args[""] @@ -145,9 +143,7 @@ def test_that_comments_are_ignored(ErtConfigWithPlugins): @pytest.mark.usefixtures("use_tmpdir") -def test_that_quotations_in_forward_model_arglist_are_handled_correctly( - ErtConfigWithPlugins, -): +def test_that_quotations_in_forward_model_arglist_are_handled_correctly(): """This is a regression test, making sure that quoted strings behave consistently. They should all result in the same. See https://github.com/equinor/ert/issues/2766""" @@ -164,7 +160,7 @@ def test_that_quotations_in_forward_model_arglist_are_handled_correctly( with open(test_config_file_name, "w", encoding="utf-8") as fh: fh.write(test_config_contents) - res_config = ErtConfigWithPlugins.from_file(test_config_file_name) + res_config = ErtConfig.with_plugins().from_file(test_config_file_name) assert res_config.forward_model_steps[0].private_args[""] == "some, thing" assert res_config.forward_model_steps[0].private_args[""] == "some stuff" @@ -215,7 +211,7 @@ def test_that_installing_two_forward_model_steps_with_the_same_name_warn(): @pytest.mark.usefixtures("use_tmpdir") def test_that_forward_model_substitution_does_not_warn_about_reaching_max_iterations( - caplog, ErtConfigWithPlugins + caplog, ): test_config_file_name = "test.ert" test_config_contents = dedent( @@ -227,7 +223,7 @@ def test_that_forward_model_substitution_does_not_warn_about_reaching_max_iterat with open(test_config_file_name, "w", encoding="utf-8") as fh: fh.write(test_config_contents) - ert_config = ErtConfigWithPlugins.from_file(test_config_file_name) + ert_config = ErtConfig.with_plugins().from_file(test_config_file_name) with caplog.at_level(logging.WARNING): ert_config.forward_model_data_to_json(0, 0, 0) assert "Reached max iterations" not in caplog.text @@ -254,7 +250,7 @@ def test_that_installing_two_forward_model_steps_with_the_same_name_warn_with_di @pytest.mark.usefixtures("use_tmpdir") -def test_that_spaces_in_forward_model_args_are_dropped(ErtConfigWithPlugins): +def test_that_spaces_in_forward_model_args_are_dropped(): test_config_file_name = "test.ert" # Intentionally inserted several spaces before comma test_config_contents = dedent( @@ -266,7 +262,7 @@ def test_that_spaces_in_forward_model_args_are_dropped(ErtConfigWithPlugins): with open(test_config_file_name, "w", encoding="utf-8") as fh: fh.write(test_config_contents) - ert_config = ErtConfigWithPlugins.from_file(test_config_file_name) + ert_config = ErtConfig.with_plugins().from_file(test_config_file_name) assert len(ert_config.forward_model_steps) == 1 job = ert_config.forward_model_steps[0] assert job.private_args.get("") == "smersion" diff --git a/tests/unit_tests/shared/share/test_shell.py b/tests/unit_tests/shared/share/test_shell.py index fcae9ad6953..6bb641cf726 100644 --- a/tests/unit_tests/shared/share/test_shell.py +++ b/tests/unit_tests/shared/share/test_shell.py @@ -439,8 +439,8 @@ def minimal_case(tmpdir): yield -def test_shell_script_jobs_availability(ErtConfigWithPlugins, minimal_case): - ert_config = ErtConfigWithPlugins.from_file("config.ert") +def test_shell_script_jobs_availability(minimal_case): + ert_config = ErtConfig.with_plugins().from_file("config.ert") fm_shell_jobs = {} for fm_step in ert_config.installed_forward_model_steps.values(): exe = fm_step.executable diff --git a/tests/unit_tests/test_run_path_creation.py b/tests/unit_tests/test_run_path_creation.py index 7a7e6a5c855..5808f3d15fe 100644 --- a/tests/unit_tests/test_run_path_creation.py +++ b/tests/unit_tests/test_run_path_creation.py @@ -332,9 +332,7 @@ def test_that_data_file_sets_num_cpu(eclipse_data, expected_cpus): "ignore:.*RUNPATH keyword contains deprecated value placeholders.*:ert.config.ConfigWarning" ) @pytest.mark.usefixtures("use_tmpdir") -def test_that_deprecated_runpath_substitution_remain_valid( - prior_ensemble, ErtConfigWithPlugins -): +def test_that_deprecated_runpath_substitution_remain_valid(prior_ensemble): """ This checks that deprecated runpath substitution, using %d, remain intact. """ @@ -348,7 +346,7 @@ def test_that_deprecated_runpath_substitution_remain_valid( ) Path("config.ert").write_text(config_text, encoding="utf-8") - ert_config = ErtConfigWithPlugins.from_file("config.ert") + ert_config = ErtConfig.with_plugins().from_file("config.ert") run_context = ensemble_context( prior_ensemble,