[BUG FIX] JSON Serialize Decimal #13050
Conversation
|
I don't think the solution is to add it to our JSON encoder. Instead, SQL sensor should not store decimals in the state machine to begin with. It's not up to us to decide that all decimal values can be represented as a float. |
|
I tried to repro it with SQLite but it never returns the decimal type. Must be a MySQL thing or something. |
|
It was the update from 0.64 to 0.65 that triggered this bug. (I can reproduce the error) Decimal is not sqlalchemy exclusive, other libs in the future can trigger the same issue, will we replicate #13059 throughout the code in the future ? |
|
We will. If the precision is important, the decimal should be converted to a string (like with money!). It's not up to us to decide that precision is not important and convert it to a float. |
|
I understand the view to ring-fence the issue to the source. But the issue at hand is related to JSON serialisation, not with internal issues (might be forgetting something here) A decimal in the state machine should be no different then of a set (which is still considered) A 2nd order sensor that could use the precision of decimal will be stripped of the possibility. |
|
All data in the state machine should be JSON serializable. (we also test this) The driver behind this is that we want to be able to have a multi-Home Assistant setup and thus need to be able to share states via JSON without having to worry about all the different types that can be int the state machine |
|
And so the issue here is not JSON serialization, it's that the SQL sensor violated this unwritten rule. |
|
But this PR actually enables the serialization of Decimal... Libraries such as Simplejson actually promote Decimal instead of float. http://simplejson.readthedocs.io/en/latest/index.html?highlight=Decimal |
|
The problem is that by enabling serialization of Decimal, you should also always deserialize to Decimal or else you still lose precision. Also, sometimes you don't care about precision (or it was never there) and you don't want people consuming your API to have to convert it to a float themselves. |
|
I've merged the other PR since it's the right solution. |
Description:
Related issue (if applicable): fixes #13021
Checklist:
tox. Your PR cannot be merged unless tests pass