Skip to content
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

Make config for different step types into separate classes #1571

Closed
sondreso opened this issue Apr 21, 2021 · 0 comments · Fixed by #1591
Closed

Make config for different step types into separate classes #1571

sondreso opened this issue Apr 21, 2021 · 0 comments · Fixed by #1591

Comments

@sondreso
Copy link
Collaborator

sondreso commented Apr 21, 2021

Currently the config for steps are vary cluttered and difficult to read. This is because the different types requires different validation, as seen below:

class Step(_StagesConfig):
name: str
type: Literal["unix", "function"]
script: Optional[List[str]] = []
input: List[InputRecord]
output: List[OutputRecord]
transportable_commands: Optional[List[TransportableCommand]] = []
function: Optional[Callable]
@root_validator
def check_defined(cls, step):
cmd_names = [cmd.name for cmd in step.get("transportable_commands", [])]
script_lines = step.get("script", [])
if step.get("type") == "function":
if cmd_names:
raise ValueError("Commands defined for a function stage")
if script_lines:
raise ValueError("Scripts defined for a function stage")
if not step.get("function"):
raise ValueError("No function defined")
return step
@validator("function", pre=True)
def function_is_valid(cls, function: str, values) -> Optional[Callable]:
step_type = values.get("type")
if step_type != "function" and function:
raise ValueError(f"Function defined for {step_type} step")
if function is None:
return None
return _import_from(function)

Suggestion is to instead make different sub-classes for the different types, and let them handle the validation.

POC:

# test.py
from typing import Union, List

from pydantic import BaseModel, Field


class BasicModel(BaseModel):
    name: str


class SubModelA(BasicModel):
    type: str = Field("A", const=True)
    some_property: int


class SubModelB(BasicModel):
    type: str = Field("B", const=True)
    some_other_property: str


class Model(BaseModel):
    l: List[Union[SubModelA, SubModelB]]


m = Model.parse_obj(
    {
        "l": [
            {"name": "a", "type": "A", "some_property": 3},
            {"name": "my_b", "type": "B", "some_other_property": "3"},
        ]
    }
)
assert isinstance(m.l[0], SubModelA)
print(m.l[0], type(m.l[0]))
assert isinstance(m.l[1], SubModelB)
print(m.l[1], type(m.l[1]))
$ python test.py
name='a' type='A' some_property=3 <class '__main__.SubModelA'>
name='my_b' type='B' some_other_property='3' <class '__main__.SubModelB'>

Interesting links:
pydantic/pydantic#503
https://github.com/samuelcolvin/pydantic/pull/469/files
https://stackoverflow.com/questions/58301364/pydantic-and-subclasses-of-abstract-class
pydantic/pydantic#619
pydantic/pydantic#265

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants