Skip to content

Commit

Permalink
Get rid of unnecessary model validation at modelsearch workflow start
Browse files Browse the repository at this point in the history
Current usage never needs this since we always pass the model object to
create_workflow. This also simplifies the current implementation. In the
future, when we want to automate this sort of input validation at
workflow start for all tools,we can revisit this more generally.
  • Loading branch information
pharmpy-dev-123 committed Sep 28, 2022
1 parent d5b6441 commit 93a11d4
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 22 deletions.
29 changes: 11 additions & 18 deletions src/pharmpy/tools/modelsearch/tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ def create_workflow(


def start(model):
validate_model(model)
return model


Expand Down Expand Up @@ -132,23 +131,17 @@ def validate_input(
except: # noqa E722
raise ValueError(f'Invalid `search_space`, could not be parsed: "{search_space}"')

validate_model(model)


def validate_model(model):
if model is None:
return

try:
cmt = model.datainfo.typeix['compartment']
except IndexError:
pass
else:
raise ValueError(
f"Invalid `model`: found compartment column {cmt.names} in dataset. "
f"This is currently not supported by modelsearch. "
f"Please remove or drop this column and try again"
)
if model is not None:
try:
cmt = model.datainfo.typeix['compartment']
except IndexError:
pass
else:
raise ValueError(
f"Invalid `model`: found compartment column {cmt.names} in dataset. "
f"This is currently not supported by modelsearch. "
f"Please remove or drop this column and try again"
)


class ModelSearchResults(Results):
Expand Down
8 changes: 4 additions & 4 deletions tests/tools/test_modelsearch.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
exhaustive_stepwise,
reduced_stepwise,
)
from pharmpy.tools.modelsearch.tool import create_workflow, validate_input, validate_model
from pharmpy.tools.modelsearch.tool import create_workflow, validate_input
from pharmpy.workflows import Workflow

MINIMAL_INVALID_MFL_STRING = ''
Expand Down Expand Up @@ -129,7 +129,7 @@ def test_reduced_stepwise_algorithm(mfl, no_of_models):
assert all(task.name == 'run0' for task in wf.output_tasks)


def test_validate_model(create_model_for_test, testdata):
def test_validate_input_model_validation(create_model_for_test, testdata):
model_code = '''$PROBLEM
$INPUT ID VISI XAT2=DROP DGRP DOSE FLAG=DROP ONO=DROP
XIME=DROP NEUY SCR AGE SEX NYH=DROP WT DROP ACE
Expand All @@ -156,8 +156,8 @@ def test_validate_model(create_model_for_test, testdata):
path=testdata / 'nonmem' / 'models' / 'mox_simulated_normal.csv'
)

with pytest.raises(ValueError):
validate_model(model)
with pytest.raises(ValueError, match='Invalid `model`'):
validate_input(MINIMAL_VALID_MFL_STRING, 'exhaustive', model=model)


def test_is_allowed():
Expand Down

0 comments on commit 93a11d4

Please sign in to comment.