Skip to content
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

Fix int(Union[str, int]) #1551

Merged
merged 2 commits into from
Sep 23, 2017
Merged

Conversation

Sterbic
Copy link
Contributor

@Sterbic Sterbic commented Aug 16, 2017

Calling int() over a union of str and int is currently broken. Fixed the types for int.__init__ to solve this and remove __int__ and __float__ from the str class since it doesn't have those methods.

Solves the following case:

from typing import Union

user_id: Union[str, int] = "123"
user_id_str = "123"
user_id_int = 123

print(int(user_id))
print(int(user_id_str))
print(int(user_id_int))

Mypy error: test.py:7: error: Argument 1 to "int" has incompatible type "Union[str, int]"; expected "SupportsInt"

@ambv
Copy link
Contributor

ambv commented Aug 21, 2017

Removal of __float__ and __int__ methods looks good. The other change works around what is apparently a bug in mypy: there's already an overload there with the types you're adding.

class int(SupportsInt, SupportsFloat, SupportsAbs[int]):
    @overload
    def __init__(self, x: SupportsInt = ...) -> None: ...
    @overload
    def __init__(self, x: Union[str, bytes], base: int = ...) -> None: ...

Since the base attribute has a default value, it seems to me int('10') should have always worked. @JukkaL should we go forward with this workaround or file and fix an issue in mypy instead?

@Sterbic
Copy link
Contributor Author

Sterbic commented Aug 29, 2017

Ping

@JukkaL
Copy link
Contributor

JukkaL commented Aug 30, 2017

The mypy issue may take a while to get fixed, so going forward with a workaround seems reasonable. We can make the workaround cleaner by tweaking the signature a bit:

class int(SupportsInt, SupportsFloat, SupportsAbs[int]):
    @overload
    def __init__(self, x: Union[str, bytes, SupportsInt] = ...) -> None: ...
    @overload
    def __init__(self, x: Union[str, bytes], base: int) -> None: ...

Note that there is no default for base, so the overload items aren't overlapping.

@gvanrossum
Copy link
Member

There's something funky with the diff. It seems you have a lot of commits in your diff that are not yours and are from other PRs. Maybe you can merge or rebase your local repo (from upstream) and then git push -f, to simplify the review process?

@Sterbic
Copy link
Contributor Author

Sterbic commented Aug 30, 2017

It seems I did a bad rebase, should be fixed now.

@JelleZijlstra JelleZijlstra merged commit 1f867d8 into python:master Sep 23, 2017
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.

None yet

5 participants