Skip to content

bytes formatting incorrect in python 3 #2070

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

Closed
euresti opened this issue Aug 31, 2016 · 7 comments
Closed

bytes formatting incorrect in python 3 #2070

euresti opened this issue Aug 31, 2016 · 7 comments
Labels
bug mypy got something wrong

Comments

@euresti
Copy link
Contributor

euresti commented Aug 31, 2016

The following runs properly:

def foo(a: bytes, b:bytes) -> bytes:
    return b'%s:%s' % (a, b)
assert foo(b'a', b'b') == b'a:b'

However mypy complains:

example.py: note: In function "foo":
example.py:2: error: Incompatible return value type (got "str", expected "bytes")
@gvanrossum gvanrossum added the bug mypy got something wrong label Aug 31, 2016
@gvanrossum gvanrossum added this to the 0.4.x milestone Aug 31, 2016
@gvanrossum
Copy link
Member

Oooh, good catch! I suspect the % checking code for bytes was too hastily adapted from the same for strings. There are many things wrong there: the checker always assumes the full formatting language used for str, while bytes have a separate (weaker) language. It always returns builtins.str as the return type (which is actually the error you're seeing). And I don't think the checker is invoked for unicode strings in PY2 mode...

@gvanrossum
Copy link
Member

gvanrossum commented Aug 31, 2016

(Also, though not affecting mypy's behavior, typeshed has no __mod__ operator for bytes defined. It's new in Python 3.5, so mypy really should check python_version too.)

@gvanrossum
Copy link
Member

@buckbaskin If you want to tackle this that's great! IIRC it should be a fairly simple refactoring and doing this will get you up to speed with how to run the tests. (Hint: pip install -r test-requirements.txt and then you can run tests with py.test, see README.md.)

@buckbaskin
Copy link
Contributor

It looks like the type checking code for bytes is just to use the string version (see checkexpr.py). The lines effectively check to see if it either matches StrExpr or BytesExpr and casts to StrExpr if it matches either one. This leads to the bug, where the BytesExpr is supposed to stay a BytesExpr.

Would the correct way to fix this be to create a new checker for Bytes? Or possibly to add a method to what is now StringFormatterChecker that checks bytes instead?

@gvanrossum
Copy link
Member

The quick and dirty fix would be to just change the return type to be the same as the arg type. But the bytes % operator supports a smaller mini-language, so technically StringFormatterChecker should grow another method to handle that. Or maybe a subclass? The main thing that ought to change seems to be conversion_type().

But I'll settle for the quick and dirty fix as a first installment!

@buckbaskin
Copy link
Contributor

In the pull request, I added a separate method that handles BytesExpr. I don't think that the type checking for what can be interpolated into a b'' is correct (it still uses the logic for a string) but the code example given in the bug no longer has an error.
On a parallel thought, is there a place where I could include a test that will check to see that something like the above example passes without error? I ran a manual check before I made the pull request.

@gvanrossum
Copy link
Member

gvanrossum commented Sep 22, 2016 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong
Projects
None yet
Development

No branches or pull requests

3 participants