Fix #2070 - bytes formatting incorrect in python 3 #2168
Conversation
|
Thanks! This looks fine, but I have a few requests:
|
- check_str_interpolation accepts StrExpr, BytesExpr, UnicodeExpr - remove a cast to a StrExpr - Add "support" for UnicodeExpr * Note that this does not implement special checks for differences between valid StrExpr and BytesExpr
| return self.strfrm_checker.check_byte_interpolation(e.left, e.right) | ||
| elif isinstance(e.left, StrExpr): | ||
| return self.strfrm_checker.check_str_interpolation(cast(StrExpr, e.left), e.right) | ||
| return self.strfrm_checker.check_str_interpolation(e.left, e.right) |
There was a problem hiding this comment.
That can't be right? What if e.left is not a string at all?
There was a problem hiding this comment.
I added the isinstance check back. It was my oversite that non StrExpr-like objects would get through to this point.
| @@ -55,7 +55,7 @@ def __init__(self, | |||
| self.exprchk = exprchk | |||
| self.msg = msg | |||
|
|
|||
| def check_str_interpolation(self, str: StrExpr, replacements: Node) -> Type: | |||
| def check_str_interpolation(self, str: Union[StrExpr, BytesExpr, UnicodeExpr], replacements: Node) -> Type: | |||
| """Check the types of the 'replacements' in a string interpolation | |||
| expression: str % replacements | |||
| """ | |||
There was a problem hiding this comment.
Can you add a TODO around here that in Python 3 the bytes format characters are actually more restricted than str/unicode format characters? (a) There are fewer supported characters; (b) %s only supports bytes. (In Python 2 these limitations do not apply.)
There was a problem hiding this comment.
Also bytes formatting in Python 3 is only supported in 3.5 and up.
There was a problem hiding this comment.
Is the bytes formatting in 3.5 a TODO? Or is it something that I'll need to check for in code?
|
Up to you. If you have time to learn how to do it it would be great to add On Sun, Sep 25, 2016 at 9:16 PM, Buck Baskin notifications@github.com
--Guido van Rossum (python.org/~guido) |
|
Thank you for the patch! Are you interested in doing any of the follow-up TODOs? |
|
I was planning on looking into the pyversion check next. What is the preferred process for completing a TODO? Should I file a bug and then make a pull request to fix it? |
|
You can just send the PR, no issue required. --Guido (mobile) On Sep 26, 2016 10:29 AM, "Buck Baskin" notifications@github.com wrote:
|
This pull request aims to fix the type checking of bytes strings. The code previously used a single path for both bytes and strings, which broke the example given in the bug (#2070) that expected a bytes output to a function for example. This pull request creates a separate method for handling bytes in the same pattern as strings, but with the correct type output.
At the time the pull request is created, there may be some errors in the types of inputs into the bytes % interpolation because it is using the type checking logic for string interpolation.