epics-containers / ibek

IOC Builder for EPICS and Kubernetes
https://epics-containers.github.io/ibek
Apache License 2.0
10 stars 4 forks source link

Support schema should have union of args with discriminator #150

Closed coretl closed 4 weeks ago

coretl commented 7 months ago

https://github.com/epics-containers/ibek/blob/54ae30d47ca12f86a084b2f37afc2e8638bcd2a8/src/ibek/support.py#L190-L192

Was expecting to see a discriminator here...

coretl commented 7 months ago

I was expecting something like:

https://github.com/epics-containers/ibek/blob/54ae30d47ca12f86a084b2f37afc2e8638bcd2a8/src/ibek/ioc.py#L175-L181

gilesknap commented 7 months ago

@coretl how did you spot that?!

I can't get the fix to work. I believe you understand the type system better than I, can you see why this https://github.com/epics-containers/ibek/blob/e4a934e7e071a0ad592ade528178e4e85991d79f/src/ibek/support.py#L194-L200

is getting this error? - it almost looks like it thinks one of the subclasses does not have 'type' but I believe they all do.

[pmac](annotated_args)[ibek]$ pytest tests
ImportError while loading conftest '/repos/ibek/tests/conftest.py'.
tests/conftest.py:13: in <module>
    from ibek.__main__ import cli  # noqa: E402
src/ibek/__main__.py:9: in <module>
    from ibek.ioc_cmds.commands import ioc_cli
src/ibek/ioc_cmds/commands.py:11: in <module>
    from ibek.gen_scripts import ioc_create_model
src/ibek/gen_scripts.py:15: in <module>
    from .ioc import IOC, Entity, clear_entity_model_ids, make_entity_models, make_ioc_model
src/ibek/ioc.py:21: in <module>
    from .support import Definition, EnumArg, IdArg, ObjectArg, Support
src/ibek/support.py:182: in <module>
    class Definition(BaseSettings):
/venv/lib/python3.10/site-packages/pydantic/_internal/_model_construction.py:92: in __new__
    private_attributes = inspect_namespace(
/venv/lib/python3.10/site-packages/pydantic/_internal/_model_construction.py:372: in inspect_namespace
    raise PydanticUserError(
E   pydantic.errors.PydanticUserError: A non-annotated attribute was detected: `arg_union = typing.Union[ibek.support.FloatArg, ibek.support.StrArg, ibek.support.IntArg, ibek.support.BoolArg, ibek.support.ObjectArg, ibek.support.IdArg, ibek.support.EnumArg]`. All model fields require a type annotation; if `arg_union` is not meant to be a field, you may be able to resolve this error by annotating it as a `ClassVar` or updating `model_config['ignored_types']`.
E
E   For further information visit https://errors.pydantic.dev/2.5/u/model-field-missing-annotation
coretl commented 7 months ago

What happens if you squash it?

discriminated = Annotated[Union[tuple(Arg.__subclasses__())], Field(discriminator="type")]  # type: ignore 
args: Sequence[discriminated] = Field(  # type: ignore 
    description="The arguments IOC instance should supply", default=() 
) 
gilesknap commented 7 months ago

different error:

[pmac](annotated_args)[ibek]$ pytest tests
ImportError while loading conftest '/repos/ibek/tests/conftest.py'.
tests/conftest.py:13: in <module>
    from ibek.__main__ import cli  # noqa: E402
src/ibek/__main__.py:9: in <module>
    from ibek.ioc_cmds.commands import ioc_cli
src/ibek/ioc_cmds/commands.py:11: in <module>
    from ibek.gen_scripts import ioc_create_model
src/ibek/gen_scripts.py:15: in <module>
    from .ioc import IOC, Entity, clear_entity_model_ids, make_entity_models, make_ioc_model
src/ibek/ioc.py:21: in <module>
    from .support import Definition, EnumArg, IdArg, ObjectArg, Support
src/ibek/support.py:182: in <module>
    class Definition(BaseSettings):
/venv/lib/python3.10/site-packages/pydantic/_internal/_model_construction.py:92: in __new__
    private_attributes = inspect_namespace(
/venv/lib/python3.10/site-packages/pydantic/_internal/_model_construction.py:372: in inspect_namespace
    raise PydanticUserError(
E   pydantic.errors.PydanticUserError: A non-annotated attribute was detected: `discriminated = typing.Annotated[typing.Union[ibek.support.FloatArg, ibek.support.StrArg, ibek.support.IntArg, ibek.support.BoolArg, ibek.support.ObjectArg, ibek.support.IdArg, ibek.support.EnumArg], FieldInfo(annotation=NoneType, required=True, discriminator='type')]`. All model fields require a type annotation; if `discriminated` is not meant to be a field, you may be able to resolve this error by annotating it as a `ClassVar` or updating `model_config['ignored_types']`.
E
E   For further information visit https://errors.pydantic.dev/2.5/u/model-field-missing-annotation
[pmac](annotated_args)[ibek]$ 
gilesknap commented 7 months ago

I'm struggling to make this work. The error seems to say 'the annotated member is not annotated'.

In IOC we are dealing with dynamic classes and it just works, even though it looks mighty similar.

Is there an issue that you are aware of that this lack of correct discrimination is causing?

coretl commented 7 months ago

This just means the schema will give less descriptive error messages (rather than type: int required int default, it will say {"type": "int", "default": "blah", ...} doesn't match .

gilesknap commented 7 months ago

OK, I'll leave this for now but keep the ticket open.

coretl commented 7 months ago

Interestingly, you can pass annotated to Field, does this work?

discriminated = Annotated[Union[tuple(Arg.__subclasses__())], Field(discriminator="type", annotated=Arg)]  # type: ignore 
args: Sequence[discriminated] = Field(  # type: ignore 
    description="The arguments IOC instance should supply", default=() 
) 
gilesknap commented 7 months ago

Pydantic is on to you!

E   pydantic.warnings.PydanticDeprecatedSince20: Using extra keyword arguments on `Field` is deprecated and will be removed. Use `json_schema_extra` instead. (Extra keys: 'annotated'). Deprecated in Pydantic V2.0 to be removed in V3.0. See Pydantic V2 Migration Guide at https://errors.pydantic.dev/2.5/migration/
gilesknap commented 4 weeks ago

We sorted this one some time ago:


    def make_ioc_model(self, entity_models: List[Type[Entity]]) -> Type[IOC]:
        """
        Create an IOC derived model, by setting its entities attribute to
        be of type 'list of Entity derived classes'.

        Also instantiate any SubEntities at this time
        """

        entity_union = Union[tuple(entity_models)]  # type: ignore
        discriminated = Annotated[entity_union, Field(discriminator="type")]  # type: ignore

        class NewIOC(IOC):
            entities: Sequence[discriminated] = Field(  # type: ignore
                description="List of entities this IOC instantiates"
            )

        return NewIOC