Skip to content

Commit

Permalink
Refactor linting argument
Browse files Browse the repository at this point in the history
  • Loading branch information
frode-aarstad committed Jan 3, 2025
1 parent 82cc0e7 commit 89593ba
Show file tree
Hide file tree
Showing 9 changed files with 35 additions and 203 deletions.
File renamed without changes.
2 changes: 1 addition & 1 deletion src/everest_models/everest_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def lint_forward_model(job: str, args: Sequence[str]) -> List[str]:
"""
return (
import_module(f"{JOBS}.fm_{job}.tasks")
.clean_parsed_data(("lint", *args), hook_call=True)
.clean_parsed_data(("--lint", *args), hook_call=True)
.errors
)

Expand Down
23 changes: 13 additions & 10 deletions src/everest_models/jobs/fm_well_swapping/parser.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
from functools import partial
from typing import Dict, Tuple

from everest_models.jobs.shared.parsers.action import SchemaAction

from ..shared.arguments import (
Parser,
add_output_argument,
add_wells_input_argument,
bootstrap_parser,
get_parser,
)
from ..shared.io_utils import load_json
from ..shared.parsers import bootstrap_parser
from ..shared.validators import (
is_gt_zero,
parse_file,
Expand All @@ -27,16 +30,16 @@ def _clean_constraint(value: str) -> Dict[str, Tuple[float, ...]]:
return {key: tuple(value.values()) for key, value in load_json(value).items()}


# TODO: Change program name to state adjuster or something more related to what it does
# omit anything to do with well
@bootstrap_parser(
schemas=SCHEMAS, # type: ignore
prog="Well Swapping",
description="Swap well operation status over multiple time intervals.",
)
def build_argument_parser(parser: Parser, lint: bool = False, **kwargs) -> Parser:
@bootstrap_parser
def build_argument_parser(lint: bool = False, **kwargs) -> Parser:
skip_type = kwargs.pop("skip_type", False)
parser.add_argument(
SchemaAction.register_models(SCHEMAS)

parser, required_group = get_parser(
description="Swap well operation status over multiple time intervals."
)

required_group.add_argument(
*_CONFIG_ARGUMENT.split("/"),
required=True,
type=partial(parse_file, schema=ConfigSchema) if not skip_type else str,
Expand Down
9 changes: 4 additions & 5 deletions src/everest_models/jobs/fm_well_swapping/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,9 @@ def clean_data(options: Namespace) -> Data:
cleaned_data = clean_data(options)
"""
errors: List[str] = []
lint_only = options.command == "lint"

def validate_exist(value: Any, argument: str):
if not (value or lint_only) and hasattr(options, argument):
if not (value or options.lint) and hasattr(options, argument):
errors.append(f"no {' '.join(argument.split('_'))}")
return value

Expand All @@ -103,13 +102,13 @@ def validate_exist(value: Any, argument: str):
)

return Data(
lint_only,
options.lint,
start_date=options.config.start_date,
iterations=iteration_limit,
priorities=priorities,
state=options.config.state,
cases=validate_exist(options.cases or options.config.cases(), "cases"),
output=None if lint_only else validate_exist(options.output, "output"),
output=None if options.lint else validate_exist(options.output, "output"),
state_duration=validate_exist(
options.config.constraints.rescale(
options.constraints["state_duration"]
Expand Down Expand Up @@ -137,7 +136,7 @@ def clean_parsed_data(
logger.error(erros)
parser.error(erros)

if data.lint_only:
if options.lint:
parser.exit()
return data

Expand Down
3 changes: 1 addition & 2 deletions src/everest_models/jobs/shared/parsers/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
from .action import SchemaAction
from .bootstrap import bootstrap_parser

__all__ = ["bootstrap_parser", "SchemaAction"]
__all__ = ["SchemaAction"]
174 changes: 0 additions & 174 deletions src/everest_models/jobs/shared/parsers/bootstrap.py

This file was deleted.

17 changes: 13 additions & 4 deletions tests/jobs/well_swapping/test_well_swapping_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import pytest
from sub_testdata import WELL_SWAPPING as TEST_DATA

from everest_models.everest_hooks import custom_forward_model_outputs
from everest_models.jobs.fm_well_swapping.cli import main_entry_point
from everest_models.jobs.shared.io_utils import load_json

Expand All @@ -13,7 +14,6 @@ def test_well_swapping_main_entrypoint_run(copy_testdata_tmpdir) -> None:
output = "well_swap_output.json"
main_entry_point(
(
"run",
"--config",
"well_swap_config.yml",
"--priorities",
Expand All @@ -34,7 +34,7 @@ def test_well_swapping_main_entrypoint_parse(copy_testdata_tmpdir) -> None:
files = tuple(Path().glob("*.*"))
with pytest.raises(SystemExit, match="0"):
main_entry_point(
("lint", "--cases", "wells.json", "--config", "well_swap_config.yml")
("--cases", "wells.json", "--config", "well_swap_config.yml", "--lint")
)
assert files == tuple(Path().glob("*.*"))

Expand All @@ -51,12 +51,21 @@ def test_well_swapping_main_entrypoint_parse_fault(
files = tuple(Path().glob("*.*"))
with pytest.raises(SystemExit, match="2"):
main_entry_point(
("lint", "-p", "priorities.json", "-c", "well_swap_config.yml")
("--lint", "-p", "priorities.json", "-c", "well_swap_config.yml")
)

assert files == tuple(Path().glob("*.*"))
_, err = capsys.readouterr()
assert (
"lint: error: argument -p/--priorities: All entries must contain the same amount of elements/indexes"
"error: argument -p/--priorities: All entries must contain the same amount of elements/indexes"
in err
)


def test_custom_forward_model_outputs_hook(capsys):
custom_forward_model_outputs(
["well_swapping -c c.yml -p p.json -cr cr.json -cs cs.json -o o.json"]
)
out, err = capsys.readouterr()
assert out == ""
assert err == ""
9 changes: 3 additions & 6 deletions tests/jobs/well_swapping/test_well_swapping_tasks.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from datetime import date, timedelta
from pathlib import Path
from typing import Any, Dict, List, Literal, NamedTuple, Optional, Tuple, TypedDict
from typing import Any, Dict, List, NamedTuple, Optional, Tuple, TypedDict

import pytest

Expand All @@ -24,7 +24,7 @@ class Options(NamedTuple):
priorities: Optional[List[Dict[str, float]]] = None
constraints: Optional[Constraints] = None
output: Optional[Path] = None
command: Literal["lint", "run"] = "lint"
lint: bool = False
iteration_limit: int = 0


Expand Down Expand Up @@ -58,7 +58,7 @@ def result(data: Dict[str, Any]) -> Data:
"options, expected",
[
pytest.param(
Options(config=ConfigSchema.model_validate(minimum_data)),
Options(config=ConfigSchema.model_validate(minimum_data), lint=True),
Data(
lint_only=True,
start_date=date(2024, 6, 3),
Expand All @@ -74,7 +74,6 @@ def result(data: Dict[str, Any]) -> Data:
),
pytest.param(
Options(
command="run",
config=ConfigSchema.model_validate(minimum_data),
cases=cases,
priorities=[
Expand All @@ -88,7 +87,6 @@ def result(data: Dict[str, Any]) -> Data:
),
pytest.param(
Options(
command="run",
config=ConfigSchema.model_validate(
{
**minimum_data,
Expand Down Expand Up @@ -131,7 +129,6 @@ def result(data: Dict[str, Any]) -> Data:
),
pytest.param(
Options(
command="run",
config=ConfigSchema.model_validate(
{
**minimum_data,
Expand Down
1 change: 0 additions & 1 deletion tests/workflows/test_well_swapping_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ def test_well_swapping_workflow(copy_testdata_tmpdir) -> None:

well_swapping(
[
"run",
"--config",
"well_swap_config.yml",
"--priorities",
Expand Down

0 comments on commit 89593ba

Please sign in to comment.