-
Notifications
You must be signed in to change notification settings - Fork 305
Explicit casting of Decimal to float for python3.3+ #39
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
Conversation
|
I can confirm the same error still exists in the master branch. I just opened an identical pull before I saw this. Lesson learned: check open pulls first. |
|
Duplicate of #27, neither of them actually fixing the problem. |
|
Patch works for me, python 3.4.0. Can you be more specific in which way this patch doesn't fix the problem, @luke-jr? |
|
See my comment on #27 #27 (comment) |
|
Reposting @luke-jr's comment (misposted?):
I had a look at it and subclassing JSONEncoder seems to be rather complex. A new Decimal serializer would need to handle all special cases as well (NaN etc). As the float serializer is called in several places, we would need to rewrite large parts of the encoder. Rounding and casting to float seems to be much more feasible and does the job just as well. Another option would be to switch to |
|
http://stackoverflow.com/a/1960649/2927053 doesn't look that complicated. The whole point of using Decimal is to avoid the float rounding - which casting to float doesn't do. |
|
That patch is ~6 years old and doesn't apply to today's structure. I only have a cursory look at it and thought that it won't be that simple to do it similarly. Glad to be proven wrong, though. :) Regarding rounding issues: I still don't understand what you mean with The usage as described in this PR appears to be fine to me. |
|
Without this the library does not work on Python 3. I understand the ideology of @luke-jr, however this PR simply allows the library to continue with the same behavior as it always has. It simply takes into account an incompatible change made in Python. I vote for merging this and if we wanted to rework the entire thing it could be a separate issue. I have been testing @alecalve's branch on Python 3.4.2 and have had no problems. |
Explicit casting of Decimal to float for python3.3+
|
This is the best fix for right now, until a better fix is written |
In python2.*, rounding a Decimal returns a float, however, in python 3, rounding a Decimal returns a Decimal. This leads to infinite loops in EncodeDecimal when decoding a response containing a float in python3 (tested with python 3.4.0).