-
Notifications
You must be signed in to change notification settings - Fork 271
Return string type as utf-8 decoded string #82
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
Im raising a |
a95c9fa
to
6172080
Compare
eth_abi/decoding.py
Outdated
try: | ||
value = data.decode("utf-8") | ||
except UnicodeDecodeError as e: | ||
raise UnicodeDecodeError( |
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.
This should probably just be the local DecodingError
from this library.
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.
Also, raise ... from e
will keep all the original decoding error details in the stack.
8949577
to
c979989
Compare
c979989
to
3af7cd0
Compare
|
||
decoder = StringDecoder() | ||
|
||
st.assume(not is_utf8_decodable(_bytes)) |
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.
Instead of this you can do a .filter()
call on the bytes strategy.
from cytoolz import compliment
@given(_bytes=st.binary(...).filter(compliment(is_utf8_decodable)), ...)
This should only give you byte strings which are not utf8 decodable.
6a5ff95
to
5ce134b
Compare
* add updates found when merging template with py-ssz and eth-abi * add wheel and wheel-windows to ci and reorg
What was wrong?
string types are return as utf-8 encoded bytes. Makes more sense to return as utf-8 decoded string type.
See #81
How was it fixed?
Added a utf-8 decoding step to the StringDecoder.
Cute Animal Picture