refactor(models)!: clarify States value accessors and widen accessor typing#2108
Merged
Conversation
…typing Rename States.has()/has_any() to has_value()/has_any_value() so the "present AND non-None value" semantic is explicit, and let pure existence be expressed via `name in states` (Mapping.__contains__). This removes the double-lookup antipattern the old name invited (closes #2105). Widen state accessor params to accept enums, not just str, advertising the StrEnum support the implementation already had (closes #2106): - States.{get_value,first,first_value,has_value,has_any_value}: str|OverkizState|OverkizAttribute - StateDefinitions.{first,has_any}: str|OverkizState CommandDefinitions and the device.supports_command wrappers already accept enums and keep their current names/locations. BREAKING CHANGE: States.has() -> has_value(), States.has_any() -> has_any_value().
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the v2 States container API to clearly separate “state exists” vs “state exists and has a non-None value”, and updates accessor type hints to explicitly support StrEnum keys (OverkizState / OverkizAttribute) for better ergonomics ahead of the 2.0 freeze.
Changes:
- Rename
States.has()→States.has_value()andStates.has_any()→States.has_any_value()to make the “non-None value” semantics explicit. - Widen
Statesaccessor parameter typing via aStateNamealias to acceptstr | OverkizState | OverkizAttribute(and widenStateDefinitions.{first,has_any}to acceptOverkizState). - Update migration/device-control docs and adjust/extend tests for the renamed API and enum keys.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
pyoverkiz/models.py |
Renames value-coupled States helpers and widens accessor typing to support enum keys. |
tests/test_models.py |
Renames tests to match new API and adds coverage for enum-key access on device.states. |
docs/migration-v2.md |
Updates migration guidance with new method names and recommends name in states for pure existence. |
docs/device-control.md |
Updates examples to use has_value / has_any_value and documents in for existence checks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced Jun 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two related, breaking v2 ergonomics fixes to the device accessor surface, before the 2.0 freeze.
#2105 —
States.has()/has_any()conflate existence with valuehas()returnedTrueonly when the state existed and had a non-Nonevalue, which pushed call sites toward a double lookup (has(x)thenget_value(x)). The two concepts are now separated:has()→has_value(),has_any()→has_any_value()— explicit about the "present AND non-None value" semantic.name in states(already provided byMapping.__contains__).Many consumers don't even need
has_valueanymore:get_value(x) == ONalready returnsFalsefor a missing state, collapsing the double lookup to one call.#2106 — Inconsistent accessor typing
State accessors were typed
str-only even thoughOverkizState/OverkizAttributeareStrEnumand worked at runtime. Signatures now advertise enum support, matching the command helpers:States.{get_value, first, first_value, has_value, has_any_value}→str | OverkizState | OverkizAttribute(theStatesclass backs bothdevice.statesanddevice.attributes).StateDefinitions.{first, has_any}→str | OverkizState.CommandDefinitionsanddevice.supports_command()already accepted enums — unchanged.Scope deliberately excludes new device-level wrappers (e.g.
device.has_state()):device.statesis a first-class container read constantly in consumers, whiledevice.supports_command()only exists as a shortcut to the burieddevice.definition.commands. Mirroring would duplicate the container API for no gain.Changes
pyoverkiz/models.py: rename + widen typing (newStateNamealias).docs/device-control.md,docs/migration-v2.md: updated to new names; documentinfor pure existence.tests/test_models.py: renamed tests + new test exercising enum keys.Breaking changes
States.has()→States.has_value()States.has_any()→States.has_any_value()Test plan
ruff check/ruff format/mypy/tycleanCloses #2105
Closes #2106