GSYE-651: Adapt heat pump validator to also allow parcial provision o… #480
Conversation
…f price parameters and validate if update_interval is not set to zero
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #480 +/- ##
==========================================
+ Coverage 66.42% 66.48% +0.05%
==========================================
Files 78 78
Lines 5147 5150 +3
Branches 851 852 +1
==========================================
+ Hits 3419 3424 +5
+ Misses 1566 1565 -1
+ Partials 162 161 -1 |
| and kwargs.get("final_buying_rate") is None | ||
| and kwargs.get("update_interval") is None): |
There was a problem hiding this comment.
I like the fix, however I think that this is or, not and. The original clause says: if not all 3 are not-None. Therefore the converted clause should say: if any of the 3 is None
There was a problem hiding this comment.
After looking at the following clause though, I think that this clause is redundant, because we raise an exception if any of the 3 is None. I would recommend to remove this completely actually.
| if not(kwargs.get("initial_buying_rate") | ||
| and kwargs.get("final_buying_rate") | ||
| and kwargs.get("update_interval")): |
There was a problem hiding this comment.
@spyrostz Sorry to drag you into this one more time.
We need this clause in order to allow the user to not provide pricing at all and let the strategy use the default values.
For this case, we do not need to validate.
There was a problem hiding this comment.
I rea-added it in the last commit
There was a problem hiding this comment.
Understood and thank you for correcting the clause! It now is correct and reflects the condition of all being None, contrary to the former implementation which evaluated if one of them was None.
…f price parameters and validate if update_interval is not set to zero