-
Notifications
You must be signed in to change notification settings - Fork 34
Migrate to cattrs for centralized model structuring #1994
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
base: v2/main
Are you sure you want to change the base?
Changes from all commits
b1499be
229f941
4b22346
67877b8
5acf843
784bb08
ea74572
3b7f9d5
c42663b
04dfb45
fd4e390
ca6ab24
89dd381
bc72514
d6a33e3
9b97555
2ae4046
32c74ec
9d71d5d
6701da1
d9f301b
6f2ae74
184f639
688a235
017b673
569325c
cb656fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| """Centralized cattrs converter for structuring Overkiz API responses.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import types | ||
| from enum import Enum | ||
| from typing import Any, Union, get_args, get_origin | ||
|
|
||
| import attr | ||
| import cattrs | ||
|
|
||
| from pyoverkiz.models import ( | ||
| CommandDefinition, | ||
| CommandDefinitions, | ||
| State, | ||
| States, | ||
| ) | ||
|
|
||
|
|
||
| def _is_primitive_union(t: Any) -> bool: | ||
| """True for unions of JSON-native types (e.g. StateType). | ||
|
|
||
| Excludes unions containing attrs classes (e.g. Definition | None) since those | ||
| need actual structuring by cattrs. | ||
| """ | ||
| origin = get_origin(t) | ||
| if origin is not Union and not isinstance(t, types.UnionType): | ||
| return False | ||
| non_none = [arg for arg in get_args(t) if arg is not type(None)] | ||
| if any(isinstance(arg, type) and attr.has(arg) for arg in non_none): | ||
| return False | ||
| # Exclude pure Optional[Enum] unions — those need the Enum structure hook. | ||
| return not all(isinstance(arg, type) and issubclass(arg, Enum) for arg in non_none) | ||
|
|
||
|
|
||
| def _make_converter() -> cattrs.Converter: | ||
| c = cattrs.Converter() | ||
|
|
||
| # JSON-native unions like StateType (str | int | float | … | None) are already the | ||
| # correct Python type after JSON parsing — tell cattrs to pass them through as-is. | ||
| c.register_structure_hook_func(_is_primitive_union, lambda v, _: v) | ||
|
|
||
| # Enums: call the constructor so UnknownEnumMixin._missing_ can handle unknown values | ||
| c.register_structure_hook_func( | ||
| lambda t: isinstance(t, type) and issubclass(t, Enum), | ||
| lambda v, t: v if isinstance(v, t) else t(v), | ||
| ) | ||
|
Comment on lines
+20
to
+47
|
||
|
|
||
| # Custom container types that take a list in __init__ | ||
| def _structure_states(val: Any, _: type) -> States: | ||
| if val is None: | ||
| return States() | ||
| return States([c.structure(s, State) for s in val]) | ||
|
|
||
| def _structure_command_definitions(val: Any, _: type) -> CommandDefinitions: | ||
| return CommandDefinitions([c.structure(cd, CommandDefinition) for cd in val]) | ||
|
|
||
| c.register_structure_hook(States, _structure_states) | ||
| c.register_structure_hook(CommandDefinitions, _structure_command_definitions) | ||
|
|
||
| return c | ||
|
|
||
|
|
||
| converter = _make_converter() | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The “primitive union passthrough” structure hook currently matches Optional[Enum] / Enum | None fields (e.g., FailureType | None, GatewayType | None) and returns the raw value unchanged, preventing the Enum hook from running. This regresses previous behavior where these fields were converted to their Enum types. Consider excluding Enum subclasses from
_is_primitive_union, and/or adding a dedicated hook for Optional[Enum] so unions containing enums are still structured into enums.