refactor: simplify normalization functions and remove redundant type casts#551
Open
BastienGimbert wants to merge 12 commits intojabesq-org:developmentfrom
Open
Conversation
…x retreive of gust attributes
…ributes-homestatus
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Contributor
Reviewer's GuideRefactors weather attribute normalization into shared helper functions, centralizes the attribute mapping, and simplifies ID handling and response extraction while removing redundant normalization and type casts from account handling code. Sequence diagram for extract_raw_data with shared weather normalizationsequenceDiagram
actor Caller
participant Extractor as extract_raw_data
participant Normalizer as normalize_weather_attributes
participant DictNorm as _normalize_dict
participant ValueNorm as _normalize_value
participant FixId as fix_id
Caller->>Extractor: extract_raw_data(resp, tag)
Extractor->>Normalizer: normalize_weather_attributes(resp.body)
Normalizer->>DictNorm: _normalize_dict(raw_data)
loop for each key,value in raw_data
DictNorm->>DictNorm: handle _id and dashboard_data
alt value is dict
DictNorm->>DictNorm: _normalize_dict(value)
else value is list
DictNorm->>ValueNorm: _normalize_value(item in list)
else primitive value
DictNorm->>ValueNorm: _normalize_value(value)
end
end
DictNorm-->>Normalizer: normalized_body
Normalizer-->>Extractor: body
alt tag is homes
Extractor->>FixId: fix_id(body.homes)
FixId-->>Extractor: homes
Extractor-->>Caller: {homes, errors}
else other tag
Extractor->>FixId: fix_id(body[tag])
FixId-->>Extractor: raw_data
Extractor-->>Caller: {tag: raw_data, errors}
end
Flow diagram for recursive weather attribute normalizationflowchart TD
A["normalize_weather_attributes(raw_data)"] --> B["_normalize_dict(raw_data)"]
subgraph DictNormalization
B --> C{iterate key,value in raw_data}
C -->|key is _id| D[copy _id to normalized]
C -->|key is dashboard_data and value is dict| E["_normalize_dict(value) and merge"]
C -->|other keys| F[lookup mapped_key in ATTRIBUTES_TO_FIX]
F --> G["_normalize_value(value)"]
G --> H[store mapped_key: normalized_value]
C -->|more items| C
C -->|done| I{normalized contains _id and not id}
I -->|yes| J[set id from _id]
I -->|no| K[skip]
J --> L[return normalized]
K --> L[return normalized]
end
subgraph ValueNormalization
G --> V1{value is dict}
V1 -->|yes| V2["_normalize_dict(value)"]
V1 -->|no| V3{value is list}
V3 -->|yes| V4[for each item call _normalize_value]
V3 -->|no| V5[return value]
end
A --> L
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Normalization is now applied to the entire
resp["body"]inextract_raw_datawhile the explicitnormalize_weather_attributescalls were removed fromaccount.update_devices; consider consolidating where normalization is expected to happen so that all device/module consumers see a consistent data shape. - By running
normalize_weather_attributesover the whole response body you may also transform non‑device structures such aserrors; consider scoping normalization to device/module payloads (e.g.,devices,modules,homes) to avoid unexpected changes to ancillary fields. - Previously
normalize_weather_attributesinaccount.pyensured_idwas always mirrored inidfor device/module dicts; with the refactor this guarantee is now tied to using the helper fromhelpers.py, so it may be worth double‑checking all call sites that rely onidbeing present without explicitly invoking normalization.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Normalization is now applied to the entire `resp["body"]` in `extract_raw_data` while the explicit `normalize_weather_attributes` calls were removed from `account.update_devices`; consider consolidating where normalization is expected to happen so that all device/module consumers see a consistent data shape.
- By running `normalize_weather_attributes` over the whole response body you may also transform non‑device structures such as `errors`; consider scoping normalization to device/module payloads (e.g., `devices`, `modules`, `homes`) to avoid unexpected changes to ancillary fields.
- Previously `normalize_weather_attributes` in `account.py` ensured `_id` was always mirrored in `id` for device/module dicts; with the refactor this guarantee is now tied to using the helper from `helpers.py`, so it may be worth double‑checking all call sites that rely on `id` being present without explicitly invoking normalization.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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 by Sourcery
Centralize and simplify normalization of weather-related API data while cleaning up redundant type casts and per-module normalization helpers.
Bug Fixes:
Enhancements: