Skip to content

Commit d7018ec

Browse files
Force type checker to use Pydantic's V1 API instead (#1432)
## Pull Request Checklist - [x] Fixes #1435 - [x] Tests added - [X] Documentation/examples added - [X] [Good commit messages](https://cbea.ms/git-commit/) and/or PR title This is a bit tricky, so I'm not 100% sure if I got this right. This is the my current understanding of the situation, as described in this previous [bug report](#984 (comment)). - Hera supports pretty much any version of Pydantic from `>=1.7,<3.0`. - Pydantic V1 and V2 has different APIs and Hera handles this differently at runtime fine. - For static type checkers, we need to pick one at type check time using `typing.TYPE_CHECKING`. This is necessary for both external type checkers like pyright and editor tooling. - The current type checker defaults to V2 APIs, even though Hera models use V1 under the hood. - This leads to wrong types and editor hints for methods like `wf.dict()`. For example, pylance would report this warning in vscode, but it's actually wrong. ``` The method "dict" in class "BaseModel" is deprecated   The `dict` method is deprecated; use `model_dump` instead. ``` Because Hera models use v1 API, `dict()` is not deprecated. If the user switch to `model_dump` as prescribed by the error, this would fail at runtime since `model_dump` doesn't actually exist on v1 models. I think the simplest solution here is to switch to v1 types explicitly? Since this is only used for Hera's base classes, there shouldn't be any impact elsewhere? ## Test Plan I can think of a few different things to test with both v1 and v2 version of pydantic. - [X] Make sure type checks pass, which I guess is covered by this CI. - [X] The [Original PR](https://github.com/argoproj-labs/hera/pull/950/files) added these conditional imports to make sure vscode can show inline hints for the constructors like WorkflowTemplate. Make sure this is OK in vscode. - [X] Make sure `dict()` doesn't show a warning. - ✅ I validated these 3 with my work project that's using Pydantic V2. - ✅ I validated v1 behaviour with this [tiny project in a gist](https://gist.github.com/jaseemabid/63e26ba25cd3e346d57ca9df791e3565). Attaching a vscode screenshot as well. <img width="772" alt="image" src="https://github.com/user-attachments/assets/955baacc-ce63-41fd-9339-b81e471284ee" /> --------- Signed-off-by: Jaseem Abid <[email protected]> Co-authored-by: Elliot Gunton <[email protected]>
1 parent 1ab6d23 commit d7018ec

File tree

4 files changed

+41
-28
lines changed

4 files changed

+41
-28
lines changed

src/hera/shared/_pydantic.py

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -16,35 +16,41 @@
1616
# users across both versions.
1717

1818
if _PYDANTIC_VERSION == 2:
19-
from pydantic.v1 import ( # type: ignore
19+
from pydantic.v1 import (
20+
BaseModel as PydanticBaseModel,
2021
Field,
2122
PrivateAttr,
2223
ValidationError,
2324
root_validator,
2425
validator,
2526
)
27+
from pydantic.v1.fields import FieldInfo
2628
else:
2729
from pydantic import ( # type: ignore[assignment,no-redef]
30+
BaseModel as PydanticBaseModel,
2831
Field,
2932
PrivateAttr,
3033
ValidationError,
3134
root_validator,
3235
validator,
3336
)
34-
35-
# TYPE_CHECKING-guarding specifically the `BaseModel` import helps the type checkers
36-
# provide proper type checking to models. Without this, both mypy and pyright lose
37-
# native pydantic hinting for `__init__` arguments.
37+
from pydantic.fields import FieldInfo # type: ignore[assignment,no-redef]
38+
39+
40+
# TYPE_CHECKING-guarding specifically the `BaseModel` import helps the type
41+
# checkers provide proper type checking to models. Without this, both mypy and
42+
# pyright lose native pydantic hinting for `__init__` arguments.
43+
#
44+
# Now there can be 2 versions of `BaseModel` at runtime, but here we must choose
45+
# one for the type checker, otherwise external type checkers as well as editors
46+
# won't be able to do much. vscode for example would always give 2
47+
# options when jumping to source instead of just picking one. Pydantic/Pylance
48+
# wouldn't be able to do much.
49+
#
50+
# Since Hera's models are still using v1 API, that's the reasonable choice.
3851
if TYPE_CHECKING:
39-
from pydantic import BaseModel as PydanticBaseModel
40-
from pydantic.fields import FieldInfo
41-
else:
42-
if _PYDANTIC_VERSION == 2:
43-
from pydantic.v1 import BaseModel as PydanticBaseModel # type: ignore
44-
from pydantic.v1.fields import FieldInfo
45-
else:
46-
from pydantic import BaseModel as PydanticBaseModel # type: ignore[assignment,no-redef]
47-
from pydantic.fields import FieldInfo
52+
from pydantic.v1 import BaseModel as PydanticBaseModel
53+
from pydantic.v1.fields import FieldInfo
4854

4955

5056
def get_fields(cls: Type[PydanticBaseModel]) -> Dict[str, FieldInfo]:
@@ -55,17 +61,6 @@ def get_fields(cls: Type[PydanticBaseModel]) -> Dict[str, FieldInfo]:
5561
return cls.__fields__ # type: ignore
5662

5763

58-
__all__ = [
59-
"BaseModel",
60-
"Field",
61-
"PrivateAttr",
62-
"PydanticBaseModel", # Export for serialization.py to cover user-defined models
63-
"ValidationError",
64-
"root_validator",
65-
"validator",
66-
]
67-
68-
6964
def get_field_annotations(cls: Type[PydanticBaseModel]) -> Dict[str, Any]:
7065
return {k: v for k, v in ChainMap(*(get_annotations(c) for c in cls.__mro__)).items()}
7166

src/hera/workflows/io/_io_mixins.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
if TYPE_CHECKING:
3434
# We add BaseModel as a parent class of the mixins only when type checking which allows it
3535
# to be used with either a V1 BaseModel or a V2 BaseModel
36-
from pydantic import BaseModel
36+
from hera.shared._pydantic import PydanticBaseModel as BaseModel
3737
else:
3838
# Subclassing `object` when using the real code (i.e. not type-checking) is basically a no-op
3939
BaseModel = object # type: ignore

src/hera/workflows/io/v2.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
if _PYDANTIC_VERSION == 2:
1212
from pydantic import BaseModel
1313

14-
class Input(InputMixin, BaseModel):
14+
class Input(InputMixin, BaseModel): # type: ignore[misc]
1515
"""Input model usable by the Hera Runner.
1616
1717
Input is a Pydantic model which users can create a subclass of. When a subclass
@@ -20,7 +20,7 @@ class Input(InputMixin, BaseModel):
2020
for the script_pydantic_io experimental feature.
2121
"""
2222

23-
class Output(OutputMixin, BaseModel):
23+
class Output(OutputMixin, BaseModel): # type: ignore[misc]
2424
"""Output model usable by the Hera Runner.
2525
2626
Output is a Pydantic model which users can create a subclass of. When a subclass

tests/test_unit/test_pydantic_io_mixins.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,24 @@ class Foo(Input):
151151
)
152152

153153

154+
def test_get_as_arguments_unannotated_v1():
155+
from hera.workflows.io.v1 import Input as InputV1
156+
157+
class Foo(InputV1):
158+
foo: int
159+
bar: str = "a default"
160+
161+
foo = Foo(foo=1)
162+
parameters = foo._get_as_arguments()
163+
164+
assert parameters == ModelArguments(
165+
parameters=[
166+
ModelParameter(name="foo", value=1),
167+
ModelParameter(name="bar", value="a default"),
168+
],
169+
)
170+
171+
154172
def test_get_as_arguments_with_pydantic_annotations():
155173
class Foo(Input):
156174
foo: Annotated[int, Field(gt=0)]

0 commit comments

Comments
 (0)