Improve enum structure #1925
Improve enum structure #1925
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the enum structure by introducing a shared UnknownEnumMixin class to eliminate duplicated _missing_ method implementations across multiple enum files. The refactoring consolidates common enum fallback behavior and logging into a reusable mixin.
Changes:
- Introduces
UnknownEnumMixininpyoverkiz/enums/base.pyto provide shared_missing_andfrom_valuemethods - Migrates 8 enum classes across 5 files to use the new mixin (UIClass, UIWidget, Protocol, FailureType, EventName, GatewayType, GatewaySubType, ExecutionType, ExecutionState, ExecutionSubType)
- Updates
pyoverkiz/enums/__init__.pyto explicitly construct__all__by filtering Enum subclasses
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| pyoverkiz/enums/base.py | New file introducing UnknownEnumMixin with shared missing and from_value methods |
| pyoverkiz/enums/ui.py | Adds UnknownEnumMixin to UIClass and UIWidget, removes logging imports and UIClass's missing method |
| pyoverkiz/enums/protocol.py | Adds UnknownEnumMixin to Protocol, removes logging imports and missing method, adds custom missing_message |
| pyoverkiz/enums/general.py | Adds UnknownEnumMixin to FailureType and EventName, removes logging imports and FailureType's missing method |
| pyoverkiz/enums/gateway.py | Adds UnknownEnumMixin to GatewayType and GatewaySubType, removes logging imports and their missing methods |
| pyoverkiz/enums/execution.py | Adds UnknownEnumMixin to ExecutionType, ExecutionState, and ExecutionSubType, removes logging imports and their missing methods |
| pyoverkiz/enums/init.py | Constructs explicit all list by filtering Enum subclasses from enum modules |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…um with labels, and implement protocol enum generation script
…type to list[str]
- Implemented `generate_ui_profiles` function to fetch and generate UIProfile enums. - Added error handling for fetching profile details. - Included detailed comments for commands and states in the generated enum. - Updated the main generation function to include UIProfile generation.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…improved type hinting
…t UI profile fetching in generate_enums
|
@copilot do a thorough review and fix all your suggestions in a new PR. Make sure the CI is green (so run |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 16 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async def generate_protocol_enum() -> None: | ||
| """Generate the Protocol enum from the Overkiz API.""" | ||
| username = os.environ["OVERKIZ_USERNAME"] | ||
| password = os.environ["OVERKIZ_PASSWORD"] |
There was a problem hiding this comment.
The script directly accesses environment variables with os.environ["OVERKIZ_USERNAME"] without providing a fallback or clear error message. If these environment variables are not set, the script will raise a KeyError with an unclear error message. Consider using os.environ.get() with a descriptive error message or adding a try-except block at the script's entry point to provide clearer instructions to users.
| async def generate_protocol_enum() -> None: | |
| """Generate the Protocol enum from the Overkiz API.""" | |
| username = os.environ["OVERKIZ_USERNAME"] | |
| password = os.environ["OVERKIZ_PASSWORD"] | |
| def _get_required_env(name: str) -> str: | |
| """Return the value of a required environment variable or raise a clear error.""" | |
| value = os.environ.get(name) | |
| if not value: | |
| raise RuntimeError( | |
| f"Missing required environment variable {name}. " | |
| "Set it before running this script." | |
| ) | |
| return value | |
| async def generate_protocol_enum() -> None: | |
| """Generate the Protocol enum from the Overkiz API.""" | |
| username = _get_required_env("OVERKIZ_USERNAME") | |
| password = _get_required_env("OVERKIZ_PASSWORD") |
|
|
||
| @unique | ||
| class Protocol(StrEnum): | ||
| class Protocol(UnknownEnumMixin, StrEnum): |
There was a problem hiding this comment.
Base classes have conflicting values for attribute 'missing': classmethod() and classmethod().
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
|
|
||
|
|
||
| @unique | ||
| class UIProfile(UnknownEnumMixin, StrEnum): |
There was a problem hiding this comment.
Base classes have conflicting values for attribute 'missing': classmethod() and classmethod().
|
|
||
| @unique | ||
| class UIClass(StrEnum): | ||
| class UIClass(UnknownEnumMixin, StrEnum): |
There was a problem hiding this comment.
Base classes have conflicting values for attribute 'missing': classmethod() and classmethod().
|
|
||
| @unique | ||
| class UIWidget(StrEnum): | ||
| class UIWidget(UnknownEnumMixin, StrEnum): |
There was a problem hiding this comment.
Base classes have conflicting values for attribute 'missing': classmethod() and classmethod().
|
|
||
|
|
||
| @unique | ||
| class UIClassifier(UnknownEnumMixin, StrEnum): |
There was a problem hiding this comment.
Base classes have conflicting values for attribute 'missing': classmethod() and classmethod().
…nt enum conversion (#1926) ## Progress Plan - [x] Add missing UIClassifier enum to exports - [x] Revert to simpler approach without explicit enum conversion (maintainer preference) - [x] Verify all linting and type checking passes - [x] Test functionality works correctly <!-- START COPILOT CODING AGENT TIPS --> --- ✨ Let Copilot coding agent [set things up for you](https://github.com/iMicknl/python-overkiz-api/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: iMicknl <1424596+iMicknl@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.