Skip to content

Conversation

dylanjw
Copy link
Contributor

@dylanjw dylanjw commented Jul 16, 2018

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

image

@dylanjw
Copy link
Contributor Author

dylanjw commented Jul 16, 2018

Im raising a UnicodeDecodeError. Should I replace that with a custom exception?

try:
value = data.decode("utf-8")
except UnicodeDecodeError as e:
raise UnicodeDecodeError(
Copy link
Member

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.

Copy link
Contributor

@carver carver Jul 16, 2018

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.

@dylanjw dylanjw force-pushed the string_decode_utf8 branch from 8949577 to c979989 Compare July 16, 2018 23:06
@dylanjw dylanjw force-pushed the string_decode_utf8 branch from c979989 to 3af7cd0 Compare July 17, 2018 16:43

decoder = StringDecoder()

st.assume(not is_utf8_decodable(_bytes))
Copy link
Member

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.

@dylanjw dylanjw force-pushed the string_decode_utf8 branch from 6a5ff95 to 5ce134b Compare July 19, 2018 23:27
@dylanjw dylanjw merged commit 7f0fc42 into ethereum:master Jul 20, 2018
pacrob added a commit to pacrob/eth-abi that referenced this pull request Dec 9, 2023
* add updates found when merging template with py-ssz and eth-abi

* add wheel and wheel-windows to ci and reorg
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants