Feature/gsye 636 #477
Feature/gsye 636 #477
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #477 +/- ##
==========================================
+ Coverage 65.95% 66.40% +0.45%
==========================================
Files 78 78
Lines 5128 5144 +16
Branches 848 851 +3
==========================================
+ Hits 3382 3416 +34
+ Misses 1589 1566 -23
- Partials 157 162 +5 |
…alidator. Corrected temperature and price validations.
gsy_framework/utils.py
Outdated
| def convert_kWh_to_W(energy_kWh, slot_length): | ||
| """Convert W power into energy in Wh (based on the duration of the market slot).""" | ||
| energy_Wh = energy_kWh * 1000.0 | ||
| return energy_Wh / (slot_length / duration(hours=1)) |
There was a problem hiding this comment.
which unit or type has the slot_length ? From the code, I see that it should be pendulum.duration, right?
If so, I propose
| def convert_kWh_to_W(energy_kWh, slot_length): | |
| """Convert W power into energy in Wh (based on the duration of the market slot).""" | |
| energy_Wh = energy_kWh * 1000.0 | |
| return energy_Wh / (slot_length / duration(hours=1)) | |
| def convert_kWh_to_W(energy_kWh: float, slot_length: duration) -> float: | |
| """Convert W power into energy in Wh (based on the duration of the market slot).""" | |
| energy_Wh = energy_kWh * 1000.0 | |
| return energy_Wh / slot_length.total_hours() |
There was a problem hiding this comment.
I added the type hints, however did not follow the total_hours suggestion, in order to be more evident on how the kWh -> W conversion is done here. Thanks!
There was a problem hiding this comment.
I just wanted to simplify the (slot_length / duration(hours=1)).
IMO we could use the features pendulum comes with.
But if you do not like this, I am convinced to leave it.
There was a problem hiding this comment.
This is correct from a software developer perspective, however it is hiding the actual calculation of the units, which we want to indicate that this is the opposite of the kW -> kWh calculation. We could of course change all these methods to use the total_hours() pendulum call, however it hides the conversion from the reader of the code IMO, and will not be clear on how this calculation is done.
| @@ -186,7 +186,7 @@ class HeatPumpSettings: | |||
| """Default values for the heat pump.""" | |||
| # range limits | |||
| MAX_POWER_RATING_KW_LIMIT = RangeLimit(0, sys.maxsize) | |||
| TEMP_C_LIMIT = RangeLimit(-273.15, 6000) | |||
| TEMP_C_LIMIT = RangeLimit(0.0, 200.0) | |||
There was a problem hiding this comment.
:-D
Finally somebody reacted to my easter egg.
|
|
||
| if not min_temp_c <= max_temp_c: | ||
| if not min_temp_c <= initial_temp_c <= max_temp_c: |
| @@ -78,8 +84,8 @@ def validate_rate(cls, **kwargs): | |||
| for buying_rate_arg_name in buying_rate_arg_names: | |||
| cls._check_range( | |||
| name=buying_rate_arg_name, value=kwargs[buying_rate_arg_name], | |||
| min_value=HeatPumpSettings.TEMP_C_LIMIT.min, | |||
| max_value=HeatPumpSettings.TEMP_C_LIMIT.max) | |||
| min_value=HeatPumpSettings.BUYING_RATE_LIMIT.initial, | |||
No description provided.