Repository navigation
MAINT: Consolidate workflow logic #1859
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import warnings | ||
| from pathlib import Path | ||
|
|
||
| from ert.runpaths import Runpaths as ErtRunpaths | ||
|
|
||
|
|
||
| def validate_casepath(casepath: Path) -> Path: | ||
| """Validate that the case path is absolute and defined in the ERT config.""" | ||
| if not casepath.is_absolute(): | ||
| casepath_str = str(casepath) | ||
| if casepath_str.startswith("<") and casepath_str.endswith(">"): | ||
| raise ValueError(f"Ert variable for casepath is not defined: {casepath}") | ||
| raise ValueError(f"'casepath' must be an absolute path. Got: {casepath}") | ||
| return casepath | ||
|
|
||
|
|
||
| def resolve_casepath( | ||
| run_paths: ErtRunpaths, | ||
| legacy_casepath: str | None, | ||
| require_sumo_casepath: bool = False, | ||
| ) -> Path: | ||
| """Resolve and validate case path from <SUMO_CASEPATH> or deprecated argument. | ||
|
|
||
| Uses <SUMO_CASEPATH> when defined and warns if deprecated <casepath> argument | ||
| is also provided. If Sumo is enabled but <SUMO_CASEPATH> is missing, an error | ||
| is raised, otherwise it falls back to <casepath> argument if provided. | ||
| """ | ||
|
|
||
| sumo_casepath = run_paths.substitutions.get("<SUMO_CASEPATH>") | ||
|
|
||
| if sumo_casepath: | ||
| if legacy_casepath: | ||
| warnings.warn( | ||
| "Providing the case path as argument is deprecated. " | ||
| "It is no longer used and can safely be removed from the workflow. " | ||
| "The case path is now read from the <SUMO_CASEPATH> variable.", | ||
| FutureWarning, | ||
| ) | ||
| return validate_casepath(Path(sumo_casepath)) | ||
|
|
||
| if legacy_casepath: | ||
| if require_sumo_casepath: | ||
| raise ValueError( | ||
| "Missing required <SUMO_CASEPATH> definition. " | ||
| "Define it in your ERT config, for example:\n" | ||
| "DEFINE <SUMO_CASEPATH> <SCRATCH>/<USER>/<CASE_DIR>" | ||
| ) | ||
| return validate_casepath(Path(legacy_casepath)) | ||
|
|
||
| raise ValueError( | ||
| "The case path could not be resolved. Please define the " | ||
| "<SUMO_CASEPATH> variable in the ERT config, for example:\n" | ||
| "DEFINE <SUMO_CASEPATH> <SCRATCH>/<USER>/<CASE_DIR>" | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,8 @@ | |
|
|
||
| from fmu.dataio import ExportPreprocessedData | ||
|
|
||
| from ._utils import resolve_casepath | ||
|
|
||
| if TYPE_CHECKING: | ||
| from ert.runpaths import Runpaths as ErtRunpaths | ||
|
|
||
|
|
@@ -50,19 +52,6 @@ | |
| """ # noqa | ||
|
|
||
|
|
||
| def main() -> None: | ||
| """Entry point from command line | ||
|
|
||
| When script is called from an ERT workflow, it will be called through the 'run' | ||
| method on the WfCopyPreprocessedData class. This context is the intended usage. | ||
| The command line entry point is still included, to clarify the difference and | ||
| for debugging purposes. | ||
| """ | ||
| parser = get_parser() | ||
| commandline_args = parser.parse_args() | ||
| copy_preprocessed_data_main(commandline_args, run_paths=None) | ||
|
|
||
|
|
||
| class WfCopyPreprocessedData(ert.ErtScript): | ||
| """A class with a run() function that can be registered as an ERT plugin. | ||
|
|
||
|
|
@@ -81,17 +70,18 @@ def run( | |
|
|
||
|
|
||
| def copy_preprocessed_data_main( | ||
| args: argparse.Namespace, run_paths: ErtRunpaths | None = None | ||
| args: argparse.Namespace, run_paths: ErtRunpaths | ||
| ) -> None: | ||
| """Copy the preprocessed data to scratch and upload it to sumo.""" | ||
|
|
||
| _normalize_workflow_arguments(args) | ||
| check_arguments(args) | ||
| logger.setLevel(args.verbosity) | ||
|
|
||
| casepath = _resolve_casepath(run_paths, args.ert_caseroot) | ||
| ert_config_path = _resolve_ert_config_path(run_paths, args.ert_config_path) | ||
| casepath = resolve_casepath(run_paths, args.ert_caseroot) | ||
| ert_config_path = Path(run_paths.substitutions["<CONFIG_PATH>"]) | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The ert config path is always set by ert in the substitutions. So this should be enough 🙂 |
||
| searchpath = ert_config_path / args.inpath | ||
|
|
||
| match_pattern = "[!.]*" # ignore metafiles (starts with '.') | ||
| files = [ | ||
| filepath | ||
|
|
@@ -126,6 +116,13 @@ def check_arguments(args: argparse.Namespace) -> None: | |
| FutureWarning, | ||
| ) | ||
|
|
||
| if args.ert_config_path: | ||
| warnings.warn( | ||
| "The argument 'ert_config_path' is deprecated. It is no longer used " | ||
| "and can safely be removed from WF_COPY_PREPROCESSED_DATAIO.", | ||
| FutureWarning, | ||
| ) | ||
|
|
||
| if Path(args.inpath).is_absolute(): | ||
| logger.debug("Argument 'inpath' is absolute: %s", args.inpath) | ||
| raise ValueError( | ||
|
|
@@ -151,74 +148,6 @@ def _normalize_workflow_arguments(args: argparse.Namespace) -> None: | |
| ) | ||
|
|
||
|
|
||
| def _validate_casepath(casepath: Path) -> Path: | ||
| """Validate that the case path is absolute and resolved.""" | ||
| if not casepath.is_absolute(): | ||
| casepath_str = str(casepath) | ||
| if casepath_str.startswith("<") and casepath_str.endswith(">"): | ||
| raise ValueError(f"Ert variable for casepath is not defined: {casepath}") | ||
| raise ValueError(f"'casepath' must be an absolute path. Got: {casepath}") | ||
| return casepath | ||
|
|
||
|
|
||
| def _resolve_casepath( | ||
| run_paths: ErtRunpaths | None, legacy_casepath: str | None | ||
| ) -> Path: | ||
| """Resolve the case path from ERT, with legacy argument fallback.""" | ||
| sumo_casepath = ( | ||
| run_paths.substitutions.get("<SUMO_CASEPATH>") if run_paths else None | ||
| ) | ||
|
|
||
| if sumo_casepath: | ||
| if legacy_casepath: | ||
| warnings.warn( | ||
| "The argument 'ert_caseroot' is deprecated. It is no longer used " | ||
| "and can safely be removed from WF_COPY_PREPROCESSED_DATAIO.", | ||
| FutureWarning, | ||
| ) | ||
| return _validate_casepath(Path(sumo_casepath)) | ||
| if legacy_casepath: | ||
| warnings.warn( | ||
| "The argument 'ert_caseroot' is deprecated. Define <SUMO_CASEPATH> " | ||
| "in the ERT config before removing it from " | ||
| "WF_COPY_PREPROCESSED_DATAIO.", | ||
| FutureWarning, | ||
| ) | ||
| return _validate_casepath(Path(legacy_casepath)) | ||
|
|
||
| raise ValueError( | ||
| "The case path could not be resolved. Please define the <SUMO_CASEPATH> " | ||
| "variable in the ERT config." | ||
| ) | ||
|
|
||
|
|
||
| def _resolve_ert_config_path( | ||
| run_paths: ErtRunpaths | None, legacy_config_path: str | None | ||
| ) -> Path: | ||
| """Resolve the ERT config path from ERT, with legacy argument fallback.""" | ||
| ert_config_path = ( | ||
| run_paths.substitutions.get("<CONFIG_PATH>") if run_paths else None | ||
| ) | ||
|
|
||
| if ert_config_path: | ||
| if legacy_config_path: | ||
| warnings.warn( | ||
| "The argument 'ert_config_path' is deprecated. It is no longer used " | ||
| "and can safely be removed from WF_COPY_PREPROCESSED_DATAIO.", | ||
| FutureWarning, | ||
| ) | ||
| return Path(ert_config_path) | ||
| if legacy_config_path: | ||
| warnings.warn( | ||
| "The argument 'ert_config_path' is deprecated. Run this workflow " | ||
| "through ERT before removing it from WF_COPY_PREPROCESSED_DATAIO.", | ||
| FutureWarning, | ||
| ) | ||
| return Path(legacy_config_path) | ||
|
|
||
| raise ValueError("The ERT config path could not be resolved from <CONFIG_PATH>.") | ||
|
|
||
|
|
||
| def get_parser() -> argparse.ArgumentParser: | ||
| """Construct parser object.""" | ||
| parser = argparse.ArgumentParser() | ||
|
|
@@ -254,7 +183,3 @@ def ertscript_workflow(config: ert.WorkflowConfigs) -> None: | |
| examples=EXAMPLES, | ||
| category="export", | ||
| ) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. see comment above. |
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| from pathlib import Path | ||
| from unittest.mock import MagicMock | ||
|
|
||
| import pytest | ||
|
|
||
| from fmu.dataio._workflows._utils import resolve_casepath, validate_casepath | ||
|
|
||
|
|
||
| def test_validate_casepath_returns_absolute_path() -> None: | ||
| casepath = Path("/tmp/scratch/user/case") | ||
|
|
||
| assert validate_casepath(casepath) == casepath | ||
|
|
||
|
|
||
| def test_validate_casepath_raises_for_relative_path() -> None: | ||
| with pytest.raises(ValueError, match="'casepath' must be an absolute path"): | ||
| validate_casepath(Path("relative/case")) | ||
|
|
||
|
|
||
| def test_validate_casepath_raises_for_unresolved_ert_variable() -> None: | ||
| with pytest.raises(ValueError, match="Ert variable for casepath is not defined"): | ||
| validate_casepath(Path("<SUMO_CASEPATH>")) | ||
|
|
||
|
|
||
| def test_resolve_casepath_uses_sumo_casepath_when_defined() -> None: | ||
| run_paths = MagicMock() | ||
| run_paths.substitutions = {"<SUMO_CASEPATH>": "/tmp/scratch/user/case"} | ||
|
|
||
| assert resolve_casepath(run_paths, legacy_casepath=None) == Path( | ||
| "/tmp/scratch/user/case" | ||
| ) | ||
|
|
||
|
|
||
| def test_resolve_casepath_warns_when_legacy_casepath_is_provided() -> None: | ||
| run_paths = MagicMock() | ||
| run_paths.substitutions = {"<SUMO_CASEPATH>": "/tmp/scratch/user/case"} | ||
|
|
||
| with pytest.warns(FutureWarning, match="Providing the case path as argument"): | ||
| resolved = resolve_casepath(run_paths, legacy_casepath="/tmp/legacy/case") | ||
|
|
||
| assert resolved == Path("/tmp/scratch/user/case") | ||
|
|
||
|
|
||
| def test_resolve_casepath_uses_legacy_casepath_when_allowed() -> None: | ||
| run_paths = MagicMock() | ||
| run_paths.substitutions = {} # missing <SUMO_CASEPATH> | ||
|
|
||
| assert resolve_casepath(run_paths, legacy_casepath="/tmp/legacy/case") == Path( | ||
| "/tmp/legacy/case" | ||
| ) | ||
|
|
||
|
|
||
| def test_resolve_casepath_raises_when_sumo_required_but_missing() -> None: | ||
| run_paths = MagicMock() | ||
| run_paths.substitutions = {} # missing <SUMO_CASEPATH> | ||
|
|
||
| with pytest.raises(ValueError, match="Missing required <SUMO_CASEPATH> definition"): | ||
| resolve_casepath( | ||
| run_paths, | ||
| legacy_casepath="/tmp/legacy/case", | ||
| require_sumo_casepath=True, | ||
| ) | ||
|
|
||
|
|
||
| def test_resolve_casepath_raises_when_no_casepath_is_available() -> None: | ||
| run_paths = MagicMock() | ||
| run_paths.substitutions = {} # missing <SUMO_CASEPATH> | ||
|
|
||
| with pytest.raises(ValueError, match="The case path could not be resolved"): | ||
| resolve_casepath(run_paths, legacy_casepath=None) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This main function seems like legacy. Most likely remnants from before we converted to the new ERT workflow plugin system.