-
-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support reading of multiple values at once #56
Conversation
-Move `kamstrup.py` to it's own module -Refactor `kamstrup.py` and add typings
Codecov ReportBase: 16.89% // Head: 73.86% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #56 +/- ##
===========================================
+ Coverage 16.89% 73.86% +56.96%
===========================================
Files 6 7 +1
Lines 290 329 +39
===========================================
+ Hits 49 243 +194
+ Misses 241 86 -155
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. |
Great work! You can consider moving kampstrup.py to its own package and publish it to PyPI even. This way others can use it even more easily. It also keeps the home assistant parts (this repo) and the kampstrup interfacing separate. Also, parentheses are only needed when catching multiple exception types. E.g., you can omit the parentheses on this line: |
I have thought about it, and I'm not entirely sure if I want to take public ownership of it, also because it's not really my code. But with the change I made in this PR, I agree it's pretty standalone. I'll give it another thought...
Thanks, I found a few more as well, fixed in 8192f0d |
Perhaps asking @bsdphk directly helps, maybe he can give his opinion? |
Mark async tests with `@pytest.mark.asyncio`
kamstrup.py
to its own modulekamstrup.py
and add typings@adabrandt for your code suggestion in bsdphk/PyKamstrup#6