Migrate to cattrs for centralized model structuring by iMicknl · Pull Request #1994 · iMicknl/python-overkiz-api

iMicknl · 2026-04-17T15:03:02Z

Summary

Alternative to #1991 — replaces per-model converter functions with a centralized cattrs Converter.

Adds pyoverkiz/converter.py with hooks for primitive unions, optional attrs classes, enums (with UnknownEnumMixin), and custom containers (States, CommandDefinitions)
Strips all converter infrastructure from models.py — models become pure @define(kw_only=True) data classes with only type annotations and defaults
Updates client.py to use converter.structure() instead of Model(**data)
Updates tests to use the converter for API-like dict structuring

Comparison with #1991

	#1991 (model-simplifications)	This PR (cattrs)
Converter location	Per-model (`_flexible_init`, `_to_list`, etc.)	Centralized `converter.py`
Model classes	Still have some converter logic	Pure data classes
Dependencies	None new	Adds `cattrs`
Forward references	Eliminated by class reordering	Not needed — cattrs resolves types
Splitting models later	Requires careful import ordering	No ordering constraints

Breaking Changes

ServerConfig.type renamed to ServerConfig.api_type.
Gateway.connectivity type changed to Connectivity | None (was non-optional).
Gateway.id and Place.id changed from mutable fields to read-only properties.
States.__getitem__ now raises KeyError instead of returning None — use States.get() for the old behavior.
CommandDefinitions.__getitem__ now raises KeyError instead of returning None — use CommandDefinitions.get() for the old behavior.
Event.setup_oid — field name unchanged (cattrs handles the setupOID → setup_oid conversion automatically).
Execution.action_group type changed from list[Action] to ActionGroup | None.
ActionGroup.label default simplified (no longer falls back to empty string).

Test plan

All 291 tests pass
ruff check passes
mypy passes
ty check passes
Compare readability and maintainability with Simplify model definitions with attrs converters and declarative patterns #1991

Copilot

Pull request overview

This PR migrates Overkiz API response structuring from per-model init/converter logic to a centralized cattrs converter, making pyoverkiz.models primarily plain attrs data classes and updating the client/tests to structure API-like dicts through the shared converter.

Changes:

Added a centralized pyoverkiz/converter.py with cattrs hooks (unknown key filtering, enums, unions, and custom containers).
Refactored pyoverkiz/models.py to remove most custom __init__ converter logic and rely on cattrs structuring + __attrs_post_init__ where needed.
Updated pyoverkiz/client.py and tests to use converter.structure(...); added cattrs as a dependency.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`pyoverkiz/converter.py`	Introduces centralized cattrs converter and structuring hooks.
`pyoverkiz/models.py`	Simplifies models/containers to attrs classes and post-init computed fields.
`pyoverkiz/client.py`	Switches API parsing to `converter.structure()` across endpoints.
`pyoverkiz/auth/factory.py`	Updates `ServerConfig` field rename usage (`type` → `api_type`).
`tests/test_models.py`	Updates model tests to structure API-like dicts via converter; adds container behavior tests.
`tests/test_ui_profile.py`	Updates UI profile tests to use structured models and converter.
`tests/test_client.py`	Updates tests for `ServerConfig.api_type` and `Action` kw-only initialization.
`pyproject.toml`	Adds `cattrs` dependency.
`uv.lock`	Locks `cattrs` and updates dependency graph accordingly.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…ation defaults - Replace manual __init__ methods with attrs converters/factories across all model classes, using _flexible_init decorator to handle unknown API keys. - Rename ServerConfig.type to api_type to avoid shadowing the builtin. Update all references in client.py, auth/factory.py, and tests. - Fix Location.__init__ bug where field() objects were used as parameter defaults instead of None (now fixed by removing manual __init__).

__getitem__ now raises KeyError on missing keys (standard Python mapping protocol). get() returns None for missing keys. __contains__ uses explicit name comparison instead of delegating to __getitem__.

Replace Any types with concrete types based on actual API payloads: metadata -> str, deleted_raw_devices_count -> int, protocol_type -> int.

Both containers used O(n) linear scans for __contains__, __getitem__, get, and select. These are called on hot paths during Home Assistant polling cycles across many devices.

…init Replace manual __init__ with declarative attrs fields and __attrs_post_init__. The ui_class and widget fields are now public, resolved at construction time from either the API kwargs or the definition fallback.

…rties

## Summary - ActionGroup `label` field now defaults to `""` via a `_to_str` converter instead of post-init fixup - Simplifies `__attrs_post_init__` to only handle id/oid resolution Depends on #1991 (model simplifications). ## Breaking changes - `ActionGroup.label` type changed from `str | None` to `str` (always a string, `None` input is converted to `""`) ## Test plan - [ ] ActionGroup tests pass with label=None, label="", and label="value"

The API returns `setupOID` which decamelizes to `setup_oid`, but the field was named `setupoid`. This caused `_flexible_init` to silently drop the value, making `Event.setup_oid` always None.

action_group uses _to_optional which can return None, but the type annotation was non-optional. Also add start_time, execution_type and execution_sub_type which are present in the API response but were silently dropped by _flexible_init.

The field uses _to_optional converter which returns None for None input, but the type annotation was non-optional Connectivity.

Definition.commands uses an inline lambda converter instead. This function was defined but never referenced anywhere.

Reorder class definitions so dependencies are defined before dependents, allowing converter helpers to reference actual classes instead of string names. Setup is moved to the bottom since it references nearly every other model. The only remaining string forward reference is Place.sub_places which is self-referential and unavoidable. Also adds section comments to group models by domain: state/command primitives, device/definition, execution/action groups, infrastructure, configuration, and UI profiles.

Replace per-model converter functions (_flexible_init, _to_list, _to_optional, _to_optional_enum) with a single cattrs Converter that handles all dict-to-model structuring. Models become pure data classes with only type annotations and defaults. - Add cattrs dependency - Add pyoverkiz/converter.py with hooks for primitive unions, optional attrs classes, enums, and container types (States, CommandDefinitions) - Strip all converter infrastructure from models.py - Update client.py to use converter.structure() instead of Model(**data) - Update tests to use converter for API-like dict structuring

cattrs 26.1.0 natively handles Optional[AttrsClass] unions, extra dict keys, and enum structuring — remove the custom hooks that duplicated this built-in behavior.

…ons for clarity

The _is_primitive_union hook was matching unions like FailureType | None and ExecutionState | None, causing those fields to pass through as raw values instead of being structured into enum instances. Exclude pure Optional[Enum] unions so the enum hook handles them correctly.

…nced functionality

… privacy and representation

…te their presence

…redundant id field

iMicknl · 2026-04-21T16:43:23Z

@copilot resolve the merge conflicts in this pull request

Co-authored-by: iMicknl <1424596+iMicknl@users.noreply.github.com>

Copilot · 2026-04-21T16:52:15Z

@copilot resolve the merge conflicts in this pull request

Merge conflicts are resolved and pushed in commit cb656fb. I merged v2/main into this branch, reconciled conflicts in pyoverkiz/client.py, pyoverkiz/models.py, and tests, and verified with lint/type checks plus full test suite (all passing).

tetienne

Love the direction here — converter.py at 64 lines vs the distributed _flexible_init + per-field converter approach is a much cleaner separation of concerns. The models are now pure data classes, which is exactly the right shape. Enum hook correctly routes through UnknownEnumMixin._missing_, and the _is_primitive_union logic handles the StateType passthrough cleanly (and the Optional[Enum] fix in 6701da1 is correct). One small blocker to fix, and a few things worth discussing.

Symmetric with _structure_states — if the API returns "commands": null for a device, iterating over None raises TypeError.

GenConverter in strict mode would raise on unknown API keys; Converter silently drops them for forward-compatibility.

The hooks used (register_structure_hook, register_structure_hook_func) have been stable since well before 23.2. A broader floor reduces friction with Home Assistant's dependency tree.

All ProtocolType fields are single-word lowercase, so decamelize is unnecessary. The comment prevents a future "consistency" fix from silently breaking things.

…tions Return False for non-str inputs instead of delegating to dict membership, which would raise TypeError for unhashable types.

Prevents silently corrupting the index by storing a State under a different name than its own.

iMicknl requested a review from tetienne as a code owner April 17, 2026 15:03

Copilot AI review requested due to automatic review settings April 17, 2026 15:03

Copilot started reviewing on behalf of iMicknl April 17, 2026 15:03 View session

iMicknl added enhancement New feature or request breaking v2 labels Apr 17, 2026

Copilot AI reviewed Apr 17, 2026

View reviewed changes

Comment thread pyoverkiz/models.py

Comment thread pyoverkiz/models.py

Comment thread pyoverkiz/models.py Outdated

Comment thread pyoverkiz/converter.py Outdated

Comment thread pyoverkiz/converter.py

iMicknl force-pushed the v2/cattrs-models branch from 2aaee1b to 1c7679b Compare April 17, 2026 15:11

iMicknl requested a review from Copilot April 18, 2026 10:35

Copilot started reviewing on behalf of iMicknl April 18, 2026 10:35 View session

Copilot AI reviewed Apr 18, 2026

View reviewed changes

Comment thread pyoverkiz/models.py

Comment thread pyoverkiz/converter.py

iMicknl added 19 commits April 19, 2026 20:29

Fix States/CommandDefinitions container semantics

229f941

__getitem__ now raises KeyError on missing keys (standard Python mapping protocol). get() returns None for missing keys. __contains__ uses explicit name comparison instead of delegating to __getitem__.

Type Event.metadata, deleted_raw_devices_count, and protocol_type

4b22346

Replace Any types with concrete types based on actual API payloads: metadata -> str, deleted_raw_devices_count -> int, protocol_type -> int.

Add dict index to CommandDefinitions and States for O(1) lookups

67877b8

Both containers used O(n) linear scans for __contains__, __getitem__, get, and select. These are called on hot paths during Home Assistant polling cycles across many devices.

Simplify DeviceIdentifier to use __attrs_post_init__ for base_device_url

784bb08

Replace _to_sub_places with _to_list("Place") forward reference

ea74572

Replace _to_server_enum and _to_api_type with _to_optional_enum

3b7f9d5

Remove unused _LOGGER and stale pylint comment

c42663b

Change Gateway.id and Place.id from mutable copies to read-only prope…

04dfb45

…rties

Fix Event.setupoid field name to match decamelize output

ca6ab24

The API returns `setupOID` which decamelizes to `setup_oid`, but the field was named `setupoid`. This caused `_flexible_init` to silently drop the value, making `Event.setup_oid` always None.

Fix Gateway.connectivity type annotation to be optional

bc72514

The field uses _to_optional converter which returns None for None input, but the type annotation was non-optional Connectivity.

Remove unused _to_command_definitions function

d6a33e3

Definition.commands uses an inline lambda converter instead. This function was defined but never referenced anywhere.

Simplify cattrs converter by removing redundant hooks

32c74ec

cattrs 26.1.0 natively handles Optional[AttrsClass] unions, extra dict keys, and enum structuring — remove the custom hooks that duplicated this built-in behavior.

Improve docstrings for _is_primitive_union and _make_converter functi…

9d71d5d

…ons for clarity

iMicknl added 7 commits April 19, 2026 20:29

Add new fields to Event, Gateway, Location, and Setup models for enha…

d9f301b

…nced functionality

Obfuscate country_code field in Location model for enhanced privacy

6f2ae74

Add UpdateCriticityLevel enum and update Gateway model to use it

184f639

Refactor dataclass fields in AuthContext and Credentials for improved…

688a235

… privacy and representation

Add default values for ui_class and widget in Device model and valida…

017b673

…te their presence

Refactor ActionGroup to use oid as the primary identifier and remove …

569325c

…redundant id field

iMicknl force-pushed the v2/cattrs-models branch from a17da1a to 569325c Compare April 19, 2026 18:31

Copilot started work on behalf of iMicknl April 21, 2026 16:43 View session

Merge v2/main and resolve conflicts

cb656fb

Co-authored-by: iMicknl <1424596+iMicknl@users.noreply.github.com>

Copilot finished work on behalf of iMicknl April 21, 2026 16:53

tetienne reviewed Apr 23, 2026

View reviewed changes

Comment thread pyoverkiz/converter.py

Comment thread pyoverkiz/converter.py

Comment thread pyproject.toml Outdated

Comment thread pyoverkiz/client.py

iMicknl added 6 commits April 24, 2026 11:05

Guard _structure_command_definitions against None input

e6f2e96

Symmetric with _structure_states — if the API returns "commands": null for a device, iterating over None raises TypeError.

Add comment explaining Converter over GenConverter choice

872abe1

GenConverter in strict mode would raise on unknown API keys; Converter silently drops them for forward-compatibility.

Lower cattrs minimum version to >=23.2

49ee925

The hooks used (register_structure_hook, register_structure_hook_func) have been stable since well before 23.2. A broader floor reduces friction with Home Assistant's dependency tree.

Add comment explaining skipped decamelize for ProtocolType

0e5fd1d

All ProtocolType fields are single-word lowercase, so decamelize is unnecessary. The comment prevents a future "consistency" fix from silently breaking things.

Guard __contains__ against non-str inputs in States and CommandDefini…

7991c41

…tions Return False for non-str inputs instead of delegating to dict membership, which would raise TypeError for unhashable types.

Validate state.name matches key in States.__setitem__

26412e1

Prevents silently corrupting the index by storing a State under a different name than its own.

iMicknl merged commit c1e56fc into v2/main Apr 24, 2026
8 checks passed

iMicknl deleted the v2/cattrs-models branch April 24, 2026 12:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to cattrs for centralized model structuring#1994

Migrate to cattrs for centralized model structuring#1994
iMicknl merged 33 commits intov2/mainfrom
v2/cattrs-models

iMicknl commented Apr 17, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

iMicknl commented Apr 21, 2026

Uh oh!

Copilot AI commented Apr 21, 2026

Uh oh!

tetienne left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

iMicknl commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Comparison with #1991

Breaking Changes

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

iMicknl commented Apr 21, 2026

Uh oh!

Copilot AI commented Apr 21, 2026

Uh oh!

tetienne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

iMicknl commented Apr 17, 2026 •

edited

Loading