Skip to content

Commit

Permalink
Add line numbers when validating everest config files
Browse files Browse the repository at this point in the history
  • Loading branch information
frode-aarstad committed Dec 18, 2024
1 parent 0e19012 commit 88025b6
Show file tree
Hide file tree
Showing 9 changed files with 107 additions and 41 deletions.
3 changes: 2 additions & 1 deletion src/everest/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
)
from .cvar_config import CVaRConfig
from .environment_config import EnvironmentConfig
from .everest_config import EverestConfig
from .everest_config import EverestConfig, EverestValidationError
from .export_config import ExportConfig
from .input_constraint_config import InputConstraintConfig
from .install_data_config import InstallDataConfig
Expand All @@ -28,6 +28,7 @@
"ControlVariableGuessListConfig",
"EnvironmentConfig",
"EverestConfig",
"EverestValidationError",
"ExportConfig",
"InputConstraintConfig",
"InstallDataConfig",
Expand Down
3 changes: 1 addition & 2 deletions src/everest/config/control_variable_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@
)
from ropt.enums import VariableType

from everest.config.validation_utils import no_dots_in_string, valid_range

from .sampler_config import SamplerConfig
from .validation_utils import no_dots_in_string, valid_range


class _ControlVariable(BaseModel):
Expand Down
63 changes: 58 additions & 5 deletions src/everest/config/everest_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import shutil
from argparse import ArgumentParser
from io import StringIO
from itertools import chain
from pathlib import Path
from typing import (
TYPE_CHECKING,
Expand Down Expand Up @@ -35,7 +36,6 @@
check_for_duplicate_names,
check_path_exists,
check_writeable_path,
format_errors,
unique_items,
validate_forward_model_configs,
)
Expand Down Expand Up @@ -96,6 +96,40 @@ def __setattr__(self, name, value):
super().__setattr__(name, value)


class EverestValidationError(ValueError):
def __init__(self):
super().__init__()
self.errors: list[tuple[ErrorDetails, tuple[int, int] | None]] = []

@property
def error(self):
return self.errors

def __str__(self):
return f"{self.errors!s}"


def _error_loc(error_dict: "ErrorDetails") -> str:
return " -> ".join(
str(e) for e in error_dict["loc"] if e is not None and e != "__root__"
)


def format_errors(validation_error: EverestValidationError) -> str:
msg = f"Found {len(validation_error.errors)} validation error{'s' if len(validation_error.errors) > 1 else ''}:\n\n"
error_map = {}
for error, pos in validation_error.errors:
if pos:
row, col = pos
key = f"line: {row}, column: {col}. {_error_loc(error)}"
else:
key = f"{_error_loc(error)}"
if key not in error_map:
error_map[key] = [key]
error_map[key].append(f" * {error['msg']} (type={error['type']})")
return msg + "\n".join(list(chain.from_iterable(error_map.values())))


class HasName(Protocol):
name: str

Expand Down Expand Up @@ -753,14 +787,33 @@ def lint_config_dict_with_raise(config: dict):
EverestConfig.model_validate(config)

@staticmethod
def load_file(config_path: str) -> "EverestConfig":
config_path = os.path.realpath(config_path)
def load_file(config_file: str) -> "EverestConfig":
config_path = os.path.realpath(config_file)

if not os.path.isfile(config_path):
raise FileNotFoundError(f"File not found: {config_path}")

config_dict = yaml_file_to_substituted_config_dict(config_path)
return EverestConfig.model_validate(config_dict)
try:
return EverestConfig.model_validate(config_dict)
except ValidationError as error:
exp = EverestValidationError()
file_content = []
with open(config_path, encoding="utf-8") as f:
file_content = f.readlines()

for e in error.errors(
include_context=True, include_input=True, include_url=False
):
if e["type"] == "literal_error":
for index, line in enumerate(file_content):
if (pos := line.find(e["input"])) != -1:
exp.errors.append((e, (index + 1, pos + 1)))
break
else:
exp.errors.append((e, None))

raise exp from error

@staticmethod
def load_file_with_argparser(
Expand All @@ -775,7 +828,7 @@ def load_file_with_argparser(
f"The config file: <{config_path}> contains"
f" invalid YAML syntax: {e!s}"
)
except ValidationError as e:
except EverestValidationError as e:
parser.error(
f"Loading config file <{config_path}> failed with:\n"
f"{format_errors(e)}"
Expand Down
25 changes: 2 additions & 23 deletions src/everest/config/validation_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@
import tempfile
from collections import Counter
from collections.abc import Sequence
from itertools import chain
from pathlib import Path
from typing import TYPE_CHECKING, TypeVar
from typing import TypeVar

from pydantic import BaseModel, ValidationError
from pydantic import BaseModel

from everest.config.install_data_config import InstallDataConfig
from everest.util.forward_models import (
Expand All @@ -18,8 +17,6 @@

from .install_job_config import InstallJobConfig

if TYPE_CHECKING:
from pydantic_core import ErrorDetails
_VARIABLE_ERROR_MESSAGE = (
"Variable {name} must define {variable_type} value either"
" at control level or variable level"
Expand Down Expand Up @@ -247,24 +244,6 @@ def check_writeable_path(path_source: str, config_path: Path):
raise ValueError(f"User does not have write access to {path}")


def _error_loc(error_dict: "ErrorDetails") -> str:
return " -> ".join(
str(e) for e in error_dict["loc"] if e is not None and e != "__root__"
)


def format_errors(error: ValidationError) -> str:
errors = error.errors()
msg = f"Found {len(errors)} validation error{'s' if len(errors) > 1 else ''}:\n\n"
error_map = {}
for err in error.errors():
key = _error_loc(err)
if key not in error_map:
error_map[key] = [key]
error_map[key].append(f" * {err['msg']} (type={err['type']})")
return msg + "\n".join(list(chain.from_iterable(error_map.values())))


def validate_forward_model_configs(
forward_model: list[str] | None, install_jobs: list[InstallJobConfig] | None
):
Expand Down
11 changes: 10 additions & 1 deletion tests/everest/functional/test_main_everest_entry.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
pytestmark = pytest.mark.xdist_group(name="starts_everest")


@pytest.mark.integration_test
def test_everest_entry_docs():
"""Test calling everest with --docs
Expand All @@ -40,6 +41,7 @@ def test_everest_entry_docs():
assert not err.getvalue()


@pytest.mark.integration_test
def test_everest_entry_manual():
"""Test calling everest with --manual"""
with capture_streams() as (out, err), pytest.raises(SystemExit):
Expand All @@ -55,6 +57,7 @@ def test_everest_entry_manual():
assert not err.getvalue()


@pytest.mark.integration_test
def test_everest_entry_version():
"""Test calling everest with --version"""
with capture_streams() as (out, err), pytest.raises(SystemExit):
Expand All @@ -64,6 +67,7 @@ def test_everest_entry_version():
assert any(everest_version in channel for channel in channels)


@pytest.mark.integration_test
def test_everest_main_entry_bad_command():
# Setup command line arguments for the test
with capture_streams() as (_, err), pytest.raises(SystemExit):
Expand All @@ -76,6 +80,7 @@ def test_everest_main_entry_bad_command():

@pytest.mark.flaky(reruns=5)
@pytest.mark.fails_on_macos_github_workflow
@pytest.mark.integration_test
def test_everest_entry_run(copy_math_func_test_data_to_tmp):
# Setup command line arguments
with capture_streams():
Expand Down Expand Up @@ -108,6 +113,7 @@ def test_everest_entry_run(copy_math_func_test_data_to_tmp):
assert status["status"] == ServerStatus.completed


@pytest.mark.integration_test
def test_everest_entry_monitor_no_run(copy_math_func_test_data_to_tmp):
with capture_streams():
start_everest(["everest", "monitor", CONFIG_FILE_MINIMAL])
Expand All @@ -120,13 +126,15 @@ def test_everest_entry_monitor_no_run(copy_math_func_test_data_to_tmp):
assert status["status"] == ServerStatus.never_run


@pytest.mark.integration_test
def test_everest_main_export_entry(copy_math_func_test_data_to_tmp):
# Setup command line arguments
with capture_streams():
start_everest(["everest", "export", CONFIG_FILE_MINIMAL])
assert os.path.exists(os.path.join("everest_output", "config_minimal.csv"))


@pytest.mark.integration_test
def test_everest_main_lint_entry(copy_math_func_test_data_to_tmp):
# Setup command line arguments
with capture_streams() as (out, err):
Expand All @@ -149,7 +157,7 @@ def test_everest_main_lint_entry(copy_math_func_test_data_to_tmp):
type_ = "(type=float_parsing)"
validation_msg = dedent(
f"""Loading config file <config_minimal.yml> failed with:
Found 1 validation error:
Found 1 validation error:
controls -> 0 -> initial_guess
* Input should be a valid number, unable to parse string as a number {type_}
Expand All @@ -161,6 +169,7 @@ def test_everest_main_lint_entry(copy_math_func_test_data_to_tmp):
@pytest.mark.fails_on_macos_github_workflow
@skipif_no_everest_models
@pytest.mark.everest_models_test
@pytest.mark.integration_test
def test_everest_main_configdump_entry(copy_egg_test_data_to_tmp):
# Setup command line arguments
with capture_streams() as (out, _):
Expand Down
6 changes: 3 additions & 3 deletions tests/everest/test_config_file_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
from unittest.mock import patch

import pytest
from pydantic_core import ValidationError
from ruamel.yaml import YAML

from everest import ConfigKeys as CK
from everest import config_file_loader as loader
from everest.config import EverestConfig
from everest.config.everest_config import EverestValidationError
from tests.everest.utils import relpath

mocked_root = relpath(os.path.join("test_data", "mocked_test_case"))
Expand Down Expand Up @@ -122,12 +122,12 @@ def test_dependent_definitions_value_error(copy_mocked_test_data_to_tmp):
def test_load_empty_configuration(copy_mocked_test_data_to_tmp):
with open("empty_config.yml", mode="w", encoding="utf-8") as fh:
fh.writelines("")
with pytest.raises(ValidationError, match="missing"):
with pytest.raises(EverestValidationError, match="missing"):
EverestConfig.load_file("empty_config.yml")


def test_load_invalid_configuration(copy_mocked_test_data_to_tmp):
with open("invalid_config.yml", mode="w", encoding="utf-8") as fh:
fh.writelines("asdf")
with pytest.raises(ValidationError, match="missing"):
with pytest.raises(EverestValidationError, match="missing"):
EverestConfig.load_file("invalid_config.yml")
24 changes: 24 additions & 0 deletions tests/everest/test_config_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import pathlib
import re
import warnings
from argparse import ArgumentParser
from pathlib import Path
from typing import Any

Expand Down Expand Up @@ -973,3 +974,26 @@ def test_warning_forward_model_write_objectives(objective, forward_model, warnin
def test_deprecated_keyword():
with pytest.warns(ConfigWarning, match="report_steps .* can be removed"):
ModelConfig(**{"report_steps": []})


def test_load_file_non_existing():
with pytest.raises(FileNotFoundError):
EverestConfig.load_file("non_existing.yml")


def test_load_file_with_errors(copy_math_func_test_data_to_tmp, capsys):
with open("config_minimal.yml", encoding="utf-8") as file:
content = file.read()

with open("config_minimal_error.yml", "w", encoding="utf-8") as file:
file.write(content.replace("generic_control", "yolo_control"))

with pytest.raises(SystemExit):
parser = ArgumentParser(prog="test")
EverestConfig.load_file_with_argparser("config_minimal_error.yml", parser)

captured = capsys.readouterr()

assert "Found 1 validation error" in captured.err
assert "line: 4, column: 11" in captured.err
assert "Input should be 'well_control' or 'generic_control'" in captured.err
8 changes: 4 additions & 4 deletions tests/everest/test_res_initialization.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from ert.config import ExtParamConfig
from ert.config.parsing import ConfigKeys as ErtConfigKeys
from everest import ConfigKeys as CK
from everest.config import EverestConfig
from everest.config import EverestConfig, EverestValidationError
from everest.simulator.everest_to_ert import (
_everest_to_ert_config_dict,
everest_to_ert_config,
Expand Down Expand Up @@ -278,15 +278,15 @@ def test_summary_default_no_opm(copy_egg_test_data_to_tmp):
),
(
{"source": None, "link": True, "target": "bar.json"},
"Input should be a valid string [type=string_type, input_value=None, input_type=NoneType]",
"Input should be a valid string",
),
(
{"source": "", "link": "false", "target": "bar.json"},
" false could not be parsed to a boolean",
),
(
{"source": "baz/", "link": True, "target": 3},
"Input should be a valid string [type=string_type, input_value=3, input_type=int]",
"Input should be a valid string",
),
],
)
Expand All @@ -313,7 +313,7 @@ def test_install_data_with_invalid_templates(
yaml.default_flow_style = False
yaml.dump(raw_config, f)

with pytest.raises(ValueError) as exc_info:
with pytest.raises(EverestValidationError) as exc_info:
EverestConfig.load_file(config_file)

assert expected_error_msg in str(exc_info.value)
Expand Down
5 changes: 3 additions & 2 deletions tests/everest/test_yaml_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@


@pytest.mark.parametrize("random_seed", [None, 1234])
def test_random_seed(random_seed):
def test_random_seed(tmp_path, monkeypatch, random_seed):
monkeypatch.chdir(tmp_path)
config = {"model": {"realizations": [0]}}
if random_seed:
config["environment"] = {"random_seed": random_seed}
Expand Down Expand Up @@ -128,7 +129,7 @@ def test_invalid_forward_model_config_files(copy_test_data_to_tmp, monkeypatch):
config_file = "valid_config_maintained_forward_models.yml"
template_path = "./templates/wellopen.jinja"
assert f"""Loading config file <{config_file}> failed with:
Found 1 validation error:
Found 1 validation error:
* Value error, job = 'add_templates'\t-c/--config = {template_config_path}
Expand Down

0 comments on commit 88025b6

Please sign in to comment.