-
Notifications
You must be signed in to change notification settings - Fork 40
feat: enhance weather attribute normalization to support wind gust data across all API endpoints #546
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: development
Are you sure you want to change the base?
feat: enhance weather attribute normalization to support wind gust data across all API endpoints #546
Changes from 9 commits
f771989
90f7cca
7f77323
e889a78
7dce6e3
6ff21df
c6cb1c1
6a77016
9d885b4
1a58e51
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 | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,6 +12,57 @@ | |||||||||
|
|
||||||||||
| LOG: logging.Logger = logging.getLogger(__name__) | ||||||||||
|
|
||||||||||
| # types for data that can be normalized (recursive structures) - to satisfy mypy | ||||||||||
| NormalizableData = dict[str, Any] | list[Any] | str | int | float | bool | None | ||||||||||
|
|
||||||||||
| ATTRIBUTES_TO_FIX: dict[str, str] = { | ||||||||||
| "firmware": "firmware_revision", | ||||||||||
| "firmware_revision": "firmware_revision", | ||||||||||
| "firmware_name": "firmware_name", | ||||||||||
| "wifi_status": "wifi_strength", | ||||||||||
| "rf_status": "rf_strength", | ||||||||||
| "Temperature": "temperature", | ||||||||||
| "Humidity": "humidity", | ||||||||||
| "Pressure": "pressure", | ||||||||||
| "CO2": "co2", | ||||||||||
| "AbsolutePressure": "absolute_pressure", | ||||||||||
| "Noise": "noise", | ||||||||||
| "Rain": "rain", | ||||||||||
| "WindStrength": "wind_strength", | ||||||||||
| "WindAngle": "wind_angle", | ||||||||||
| "GustStrength": "gust_strength", | ||||||||||
| "GustAngle": "gust_angle", | ||||||||||
| "wind_gust": "gust_strength", | ||||||||||
| "wind_gust_angle": "gust_angle", | ||||||||||
| } | ||||||||||
|
|
||||||||||
|
|
||||||||||
| def normalize_weather_attributes(raw_data: NormalizableData) -> NormalizableData: | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you sure about the return type?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought making a return type would satisfy mypy. Otherwise, it would block since the return type wasn't explicitly specified, and "any" would also block it. At least in local.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What type does
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From this logic, it returns the same type that it received in input. If we call That's why I declare the type
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Take a look at the original function and compare how you changed the types and the internal behaviour.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Also the normalize function needs to be split so that it uses your extended version when calling from within. |
||||||||||
| """Normalize weather-related attributes recursively.""" | ||||||||||
|
|
||||||||||
| if isinstance(raw_data, list): | ||||||||||
| return [normalize_weather_attributes(item) for item in raw_data] | ||||||||||
|
|
||||||||||
| if not isinstance(raw_data, dict): | ||||||||||
| return raw_data | ||||||||||
|
|
||||||||||
| normalized: dict[str, Any] = {} | ||||||||||
| for key, value in raw_data.items(): | ||||||||||
| if key == "_id": | ||||||||||
| normalized["_id"] = value | ||||||||||
| continue | ||||||||||
| if key == "dashboard_data" and isinstance(value, dict): | ||||||||||
| # useless cast here to satisfy mypy | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This hints at some type mismatch. A cast should not be necessary.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with you that the cast is completly unnecessary. But as said, it's only to satisfy mypy. Without this cast, it blocks. I don't know what else to do in that case 😬 I'll check if there seems to be a sort of mismatch in the types.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Checked it again, and I really don't see how I can do it in another way. From what I understand, it seems that mypy expects an explicit type because it needs to guarantee that the passed object (the result of |
||||||||||
| normalized |= cast("dict[str, Any]", normalize_weather_attributes(value)) | ||||||||||
| continue | ||||||||||
|
|
||||||||||
| mapped_key = ATTRIBUTES_TO_FIX[key] if key in ATTRIBUTES_TO_FIX else key # noqa: SIM401 | ||||||||||
| normalized[mapped_key] = normalize_weather_attributes(value) | ||||||||||
|
|
||||||||||
| if "_id" in normalized and "id" not in normalized: | ||||||||||
| normalized["id"] = normalized["_id"] | ||||||||||
| return normalized | ||||||||||
|
|
||||||||||
|
|
||||||||||
| def fix_id(raw_data: list[RawData | str]) -> list[RawData | str]: | ||||||||||
| """Fix known errors in station ids like superfluous spaces.""" | ||||||||||
|
|
@@ -43,20 +94,23 @@ def extract_raw_data(resp: RawData, tag: str) -> RawData: | |||||||||
| msg = "No device found, errors in response" | ||||||||||
| raise NoDeviceError(msg) | ||||||||||
|
|
||||||||||
| # useless cast here again to satisfy mypy | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This hints at some type mismatch. A cast should not be necessary. |
||||||||||
| body = cast("dict[str, Any]", normalize_weather_attributes(resp["body"])) | ||||||||||
|
|
||||||||||
| if tag == "homes": | ||||||||||
| homes: list[dict[str, Any] | str] = fix_id(resp["body"].get(tag)) | ||||||||||
| homes: list[dict[str, Any] | str] = fix_id(body.get(tag, [])) | ||||||||||
| if not homes: | ||||||||||
| LOG.debug("Server response (tag: %s): %s", tag, resp) | ||||||||||
| msg = "No homes found" | ||||||||||
| raise NoDeviceError(msg) | ||||||||||
| return { | ||||||||||
| tag: homes, | ||||||||||
| "errors": resp["body"].get("errors", []), | ||||||||||
| "errors": body.get("errors", []), | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if not (raw_data := fix_id(resp["body"].get(tag))): | ||||||||||
| if not (raw_data := fix_id(body.get(tag, []))): | ||||||||||
| LOG.debug("Server response (tag: %s): %s", tag, resp) | ||||||||||
| msg = "No device data available" | ||||||||||
| raise NoDeviceError(msg) | ||||||||||
|
|
||||||||||
| return {tag: raw_data, "errors": resp["body"].get("errors", [])} | ||||||||||
| return {tag: raw_data, "errors": body.get("errors", [])} | ||||||||||
Uh oh!
There was an error while loading. Please reload this page.