Return usage.get_{monthstat,daystat} in expected format #394
Conversation
Codecov ReportBase: 78.95% // Head: 79.28% // Increases project coverage by
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## master #394 +/- ##
==========================================
+ Coverage 78.95% 79.28% +0.32%
==========================================
Files 25 25
Lines 1863 1868 +5
Branches 269 266 -3
==========================================
+ Hits 1471 1481 +10
+ Misses 358 353 -5
Partials 34 34
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
rytilahti
left a comment
rytilahti
left a comment
There was a problem hiding this comment.
Looking great, thanks! 💯 I added a couple of comments for your consideration prior merging this.
rytilahti
changed the title
|
Hopefully I've made changes based on all the feedback. The only thing I haven't done is added additional tests to test_usage.py. This is definitely a good idea but, as I understand it, would require extending newfakes which I think is probably outside this PR. I also feel I'll need to understand newfakes a bit better before having a go, so any hints would be great. |
|
I think it's not worth trying to force the tests inside the existing ones, but rather add module level tests separately, especially as the fixture files do not contain the necessary information. This reminded me about that I was exploring this idea earlier, see #404 if you wish. Anyway, it is good as it is also without that. I'll do a review later tonight/tomorrow. |
rytilahti
left a comment
rytilahti
left a comment
There was a problem hiding this comment.
Looks good to me after fixing some minor changes to the docs 👍
Change usage module get_daystat and get_monthat to return dictionaries of date index: time values as spec'd instead of raw usage data. Output matches emeter module get_daystat and get_monthstat
Use the new _convert function in emeter for all conversions rather than the one in smartdevice.py Removed unused function _emeter_convert_emeter_data from smartdevice.py
Tidied up some doc string comments Added a check for explicit values from conversion function
rytilahti
force-pushed
the
usage-get-stat
branch
from
850fad4 to
85f16c6
Compare
|
Uhh, I should've used merge instead of rebase to save the review comments, oh well... I rebased this on top of the current master and changed the docstrings a bit, I think this is good to go for the next release. Thanks again @jules43! 🥇 |
rytilahti
changed the title
3 participants
This is an initial fix for the issue discussed here:
Fixes #373
This code largely duplicates the solution used for the Emeter module.
I have work in progress on a pydantic based version for which I will create a second PR.