add ensure_binary/str/text helper functions - #204
Conversation
benjaminp
left a comment
There was a problem hiding this comment.
Thanks for the PR. A few things:
documentation/index.rstneeds to be updated.- There need to be tests for the second and third parameters to each function.
- The tests should actually check the type of what they're getting back from the functions under test, since converting between types is the main point of these functions.
|
|
||
|
|
||
| def ensure_str(s, encoding='utf-8', errors='strict'): | ||
| """ A helper function to ensure output is six.str_type. |
There was a problem hiding this comment.
good catch, will update the doc to six.string_types and rename function to ensure_string
| raise TypeError("not expecting type '%s'" % type(s)) | ||
| if isinstance(s, text_type): | ||
| return s.encode(encoding, errors) | ||
| else: |
There was a problem hiding this comment.
This can be elif isinstance(s, binary_type): and then the first if statement in this function may be removed.
There was a problem hiding this comment.
@benjaminp do you mean something like?
def ensure_binary(s, encoding='utf-8', errors='strict'):
if isinstance(s, text_type):
return s.encode(encoding, errors)
elif isinstance(s, binary_type):
return s
else:
raise TypeError("not expecting type '%s'" % type(s))
I'd like to keep the raise TypeError part to fail the error loudly; be consistent with the raise behavior from ensure_string.
| raise TypeError("not expecting type '%s'" % type(s)) | ||
| if isinstance(s, binary_type): | ||
| return s.decode(encoding, errors) | ||
| else: |
There was a problem hiding this comment.
The isinstance checks can be rewritten in a way similar to what I suggested in ensure_binary.
There was a problem hiding this comment.
sure, will update to the following if we agree to keep raise exception part
if isinstance(s, binary_type):
return s.decode(encoding, errors)
elif isinstance(s, text_type):
return s
else:
raise TypeError("not expecting type '%s'" % type(s))
|
Thanks for the review, @benjaminp
|
- add checking for type in tests; - fix six.str_types typo; - simplify logic in ensure_;
benjaminp
left a comment
There was a problem hiding this comment.
Almost there. There still could be some more testing of errors and encoding. e.g., what happens when invalid utf-8 is given with error="strict|replace|igonre"?
| .. function:: ensure_binary(s, encoding='utf-8', errors='strict') | ||
|
|
||
| A helper function to ensure output is :data:`binary_type`. ``encoding``, ``errors`` | ||
| are the same as https://docs.python.org/3/library/stdtypes.html#str.encode |
There was a problem hiding this comment.
Write
:meth:`py3:str.encode`
here and below
| assert getattr(six.moves.builtins, 'bytes', str)(my_test) == six.b("hello") | ||
|
|
||
|
|
||
| class EnsureTests(unittest.TestCase): |
There was a problem hiding this comment.
Don't need to inherit from TestCase here.
|
Hi! @benjaminp, @jz1371 is it moving forward? @jz1371 if you have no time I am happy to finish it. You can see there is a need for these functions in the community. They even made it to a keynote at PyCon 🤣 https://youtu.be/66XoCk79kjM?t=31m15s |
|
@benjaminp, any other changes you'd like to have in this PR? |
|
THIS IS SO AWESOME!! I was watching the Pycon 2017 Instagram video and thinking "This needs to be in six!" and it is already. Any sense on when this will be in a release? |
|
Indeed. I am looking forward to remove this from m codebase :) |
python-autopxd already requires six, but the new `ensure_binary` function is only in the git repo and hasn't been released to PyPI yet, so I temporarily copied it here. The source comes from: benjaminp/six#204 It's a trivial function (every py2.py3 project has its own way to do the same thing). Fixes tarruda#12 Supersedes tarruda#3
|
It would be great if we could have this released to pypi some time soon, thanks! |
|
@benjaminp Hello there! Any idea when this may be released? |
|
six==1.14 will work with the error 'module 'six' has no attribute 'ensure_str'' |
|
six==1.14.0 (with the .0) as in https://pypi.org/project/six/
|

Summary
add ensure_binary, ensure_str, ensure_text helper functions to solve the encoding compatibility
between py2 and py3 more easily.
-- ensure_binary, input --> six.binary_type
-- ensure_str, input -> six.string_type
-- ensure_text, input -> six.text_type
This is mentioned in Lisa Guo's keynote in Pycon 2017 (from 31:21 in https://www.youtube.com/watch?v=66XoCk79kjM), which helped Instagram to upgrade to Python 3 and I hope this can help other developers, too
Test Plan
add new unit tests to cover added code