Add unit conversion tests #39
Conversation
- Added test_unit_conversion test for direct unit conversion testing - Added test_get_history_with_unit_conversion to test unit conversion in get_history flow - Added test_get_history_no_conversion_needed to test when units match - Updated convert_units to work with list format from HA API - All 6 tests passing successfully
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive unit tests for the unit conversion functionality in PredAI, enabling proper handling of different measurement units (kWh↔Wh, W↔kW) when retrieving historical sensor data.
Key Changes:
- Added three new test methods to validate unit conversion functionality
- Implemented the
convert_unitsfunction to handle conversions between supported units - Updated
get_historyfunction to accept arequired_unitsparameter and perform automatic unit conversion
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| test_predai.py | Adds three new unit tests: test_unit_conversion validates direct unit conversions, test_get_history_with_unit_conversion tests W→kW conversion in the get_history flow, and test_get_history_no_conversion_needed verifies behavior when units already match |
| predai/rootfs/predai.py | Implements convert_units function with support for kWh↔Wh and W↔kW conversions, adds required_units parameter to get_history, and integrates unit conversion into the main data retrieval flow |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """ | ||
|
|
||
| print("********* Starting PredAI *********") | ||
| print("********* Starting PredAI *********") |
There was a problem hiding this comment.
This change appears to be an unintentional whitespace modification. Two spaces were removed from the banner message, changing "Starting PredAI" alignment. This seems unrelated to the unit conversion feature being added.
| print("********* Starting PredAI *********") | |
| print("********* Starting PredAI *********") |
| # Keep original if conversion fails | ||
| pass |
There was a problem hiding this comment.
The error handling silently ignores conversion failures without logging any information. When a ValueError or KeyError occurs, the original value is kept but no warning is printed. This makes debugging difficult if data is malformed. Consider adding a print statement or warning when conversion fails to maintain consistency with the warning message on line 380 for unsupported conversions.
| # Keep original if conversion fails | |
| pass | |
| # Keep original if conversion fails, but log a warning for visibility | |
| print( | |
| "Warn: Failed to convert state '{}' from {} to {}. Keeping original value.".format( | |
| item.get("state"), from_units, to_units | |
| ) | |
| ) |
| dataset, start, end = await interface.get_history(sensor_name, now, days=days) | ||
| if required_units: | ||
| units = await interface.get_state(sensor_name, attribute="unit_of_measurement") | ||
| if (required_units is not None) and (units is not None) and (units != required_units): |
There was a problem hiding this comment.
The condition (required_units is not None) is redundant here. The outer if required_units: on line 400 already checks this condition, so this inner check is unnecessary and reduces code clarity.
| if (required_units is not None) and (units is not None) and (units != required_units): | |
| if (units is not None) and (units != required_units): |
| async def convert_units(dataset, from_units, to_units): | ||
| """ | ||
| Convert units in the dataset from from_units to to_units. | ||
| Currently supports conversion between kWh and Wh. |
There was a problem hiding this comment.
The documentation comment is incomplete and outdated. It states the function "Currently supports conversion between kWh and Wh" but the implementation also supports W↔kW conversions. The documentation should be updated to reflect all supported conversions: kWh↔Wh and W↔kW.
| Currently supports conversion between kWh and Wh. | |
| Currently supports conversion between kWh and Wh, and between W and kW (in both directions). |
| Handles list format from HA API. | ||
| """ | ||
| print("Converting units from {} to {}".format(from_units, to_units)) | ||
| factor = 1.0 |
There was a problem hiding this comment.
This assignment to 'factor' is unnecessary as it is redefined before this value is used.
This assignment to 'factor' is unnecessary as it is redefined before this value is used.
This assignment to 'factor' is unnecessary as it is redefined before this value is used.
This assignment to 'factor' is unnecessary as it is redefined before this value is used.
| factor = 1.0 |
| sys.path.insert(0, os.path.join(os.path.dirname(__file__), 'predai', 'rootfs')) | ||
|
|
||
| from predai import HAInterface, Prophet, timestr_to_datetime | ||
| from predai import HAInterface, Prophet, timestr_to_datetime, convert_units, get_history |
There was a problem hiding this comment.
Import of 'timestr_to_datetime' is not used.
| from predai import HAInterface, Prophet, timestr_to_datetime, convert_units, get_history | |
| from predai import HAInterface, Prophet, convert_units, get_history |
Summary
Adds comprehensive unit tests for the unit conversion functionality in PredAI.
Changes
Technical Details
convert_unitsfunction to work exclusively with list format (as returned by HA API)stateandlast_updatedfieldsTesting
All 6 tests passing: