Skip to content
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

Fix TypeError #21734

Merged
merged 5 commits into from Mar 9, 2019
Merged
Diff settings

Always

Just for now

@@ -6,6 +6,7 @@
"""
import asyncio
from datetime import timedelta
from decimal import Decimal
from functools import partial
import logging

@@ -351,7 +352,8 @@ def state(self):
# Recalculate the rate
diff = current_reading - self._previous_reading
timediff = timestamp - self._previous_timestamp
self._state = diff / timediff * 3600
total_seconds = timediff.total_seconds()
This conversation was marked as resolved by wburgers

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot Mar 8, 2019

local variable 'total_seconds' is assigned to but never used

self._state = round(diff / Decimal(total_seconds) * 3600, 3)
This conversation was marked as resolved by wburgers

This comment has been minimized.

Copy link
@amelchio

amelchio Mar 8, 2019

Contributor

Why do we need Decimal() here?

This comment has been minimized.

Copy link
@wburgers

wburgers Mar 8, 2019

Author Contributor

Because dividing a decimal by a float is not acceptable to python either apparently.
(total_seconds is a float).

This comment has been minimized.

Copy link
@amelchio

amelchio Mar 9, 2019

Contributor

Ah okay. I seem unable to remember that total_seconds() is float and so I tested with integer division.

You could avoid the import by using float(diff) / total_seconds but I guess the current solution is okay.

This comment has been minimized.

Copy link
@wburgers

wburgers Mar 9, 2019

Author Contributor

Changed it :)
(Also tested with tox and as custom_component)

This comment has been minimized.

Copy link
@WoLpH

WoLpH Mar 9, 2019

Contributor

Little bit of nitpickery, but I think the Decimal() should only be around the total_seconds. It probably won't make much difference here but you're doing floating point calculations only to convert it to decimal after. So you lose precision that way :)

This comment has been minimized.

Copy link
@amelchio

amelchio Mar 9, 2019

Contributor

@WoLpH It sounds like you have misread.

This comment has been minimized.

Copy link
@WoLpH

WoLpH Mar 9, 2019

Contributor

@amelchio oops... you're right, one of my github extensions drew a character limit line right on top of the ). My bad ;)


self._previous_reading = current_reading
self._previous_timestamp = timestamp
@@ -6,6 +6,7 @@
"""

import asyncio
import datetime
from decimal import Decimal
from unittest.mock import Mock

@@ -104,8 +105,8 @@ def test_derivative():

entity.telegram = {
'1.0.0': MBusObject([
{'value': 1551642213},
{'value': 745.695, 'unit': 'm3'},
{'value': datetime.datetime.fromtimestamp(1551642213)},
{'value': Decimal(745.695), 'unit': 'm3'},
])
}
yield from entity.async_update()
@@ -115,13 +116,13 @@ def test_derivative():

entity.telegram = {
'1.0.0': MBusObject([
{'value': 1551642543},
{'value': 745.698, 'unit': 'm3'},
{'value': datetime.datetime.fromtimestamp(1551642543)},
{'value': Decimal(745.698), 'unit': 'm3'},
])
}
yield from entity.async_update()

assert abs(entity.state - 0.03272) < 0.00001, \
assert abs(entity.state - Decimal(0.033)) < 0.00001, \
'state should be hourly usage calculated from first and second update'

assert entity.unit_of_measurement == 'm3/h'
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.