-
Notifications
You must be signed in to change notification settings - Fork 520
Conversation
Why make this so convoluted, since we're passing this to |
@@ -89,7 +89,14 @@ def _escape_value(value): | |||
elif isinstance(value, integer_types) and not isinstance(value, bool): | |||
return str(value) + 'i' | |||
elif _is_float(value): | |||
return repr(value) | |||
reprvalue = repr(value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just make this return repr(float(value))
since you've already passed the _is_float
test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I went that way because I didn't want to lose precision of the Decimal, but I don't know if it's worth it, your solution is quite shorter and more elegant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're referring to your example earlier, even if Decimal resolves to a more granular value than what you sent it may not be what you actually want. If you wanted that value down to the full granularity you should provide that to begin with (IMHO of course).
Do you want to take a stab at changing your PR? If so, can you please add a quick test to check both a non-Decimal and Decimal value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify, since your Decimal
value is set to 16 digits, you should get the repr
value of the 16-digit float instead of a (potentially) longer, more granular definition.
In [1]: from decimal import Decimal
In [2]: str(Decimal(0.8289445733333332))
Out[2]: '0.828944573333333156739399782964028418064117431640625'
In [3]: repr(Decimal(0.8289445733333332))
Out[3]: "Decimal('0.828944573333333156739399782964028418064117431640625')"
In [4]: repr(float(Decimal(0.8289445733333332)))
Out[4]: '0.8289445733333332'
…precision (influxdata#813) * chore(line_protocol): update repr value of floats to properly handle precision. Closes influxdata#488 * chore(line_protocol): fix repr and handle boolean values * chore(CHANGELOG): update to include reference to PR#488
Given the issue #431 fixing a precision issue while using python 2.x, as demonstrated by the example below:
We have now an issue as described in #430 and #482 with both Python and Python3 and the use of both Decimal Python type and repr, because :
In Python 3, str() and repr() now provides the same result with a float:
But the problems remain while using both Decimal and repr(). The pull request offers a solution which should work both for python2 and python3.
Regards,
Carl