Repository navigation
Return usage.get_{monthstat,daystat} in expected format - #394
rytilahti merged 5 commits into
Conversation
Sorry, something went wrong.
Codecov ReportBase: 78.95% // Head: 79.28% // Increases project coverage by +0.32% 🎉
📣 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. |
Sorry, something went wrong.
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.
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
rytilahti
left a comment
There was a problem hiding this comment.
Looks good to me after fixing some minor changes to the docs 👍
Sorry, something went wrong.
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
|
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! 🥇 |
Sorry, something went wrong.
) * Basic fix for issue: python-kasa#373 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 * Fixed some formatting and lint warnings to comply with black/flake8 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 * Added a first pass test module for testing the new usage conversion function * Changes based on PR feedback Tidied up some doc string comments Added a check for explicit values from conversion function * Rebase on top of current master, fix docstrings --------- Co-authored-by: Teemu Rytilahti <tpr@iki.fi>
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.