-
Notifications
You must be signed in to change notification settings - Fork 75
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 loss of precision decimal numbers #179
Conversation
… to float previously
… later conversion to str to avoid case of lost decimals
review done at the meeting |
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.
@dudzicz, small changes needed
ethtx/models/decoded_model.py
Outdated
@@ -53,6 +59,12 @@ class Argument(BaseModel): | |||
type: str | |||
value: Any | |||
|
|||
@validator("value") | |||
def decimal_conv(cls, v): |
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.
types missing
ethtx/models/decoded_model.py
Outdated
|
||
from ethtx.models.base_model import BaseModel | ||
from ethtx.models.objects_model import BlockMetadata | ||
from ethtx.models.semantics_model import AddressSemantics, ERC20Semantics | ||
|
||
# Set decimal operation precision to avoid loss of digits with arithmetic operations | ||
getcontext().prec = 256 |
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.
maybe lets move it to module where BaseModel
is declared
return "".join(new_str_format) | ||
else: | ||
return val_str | ||
else: |
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.
the same ^
|
||
new_str_format = ["0.0", "0" * num_zeros, digit_part] | ||
return "".join(new_str_format) | ||
else: |
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.
else
not needed
@@ -195,3 +201,25 @@ def create_transformation_context( | |||
context["__print_input__"] = decode_call | |||
|
|||
return context | |||
|
|||
|
|||
def _handle_decimal_representations(val): |
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.
can you add types?
.gitignore
Outdated
@@ -110,5 +110,4 @@ dmypy.json | |||
.pytype/ | |||
|
|||
# Cython debug symbols | |||
cython_debug/ | |||
|
|||
cython_debug/ |
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.
git rollback, delete all changes for this file
…osest to the values in Argument object. Moved it from base_model.py to decode_model.py.
LGTM! |
Fixed the issue of loss of precision in the decimal representation of large
floats
. Converted float toDecimals
and changed the precision of the context to 256 i.e.getcontext().prec = 256
. Eventually returns the string representation of the Decimal object.This fixes the issue of scientific notation representation of
floats
both in Argument value field and also other fields in models fromdecoded_model.py