Skip to content

Add __new__ to str and int stubs in both Pythons. - #1352

Merged
JelleZijlstra merged 3 commits into
python:masterfrom
JetBrains:sproshev/str_int_new
Jun 13, 2017
Merged

JelleZijlstra merged 3 commits into
python:masterfrom
JetBrains:sproshev/str_int_new

Conversation

@sproshev

Copy link
Copy Markdown
Contributor

No description provided.

@JelleZijlstra JelleZijlstra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What concrete problem does this solve? I'm hesitant to change stubs as basic as these without a clear justification.

Comment thread stdlib/3/builtins.pyi Outdated
@overload
def __init__(self, x: SupportsInt = ...) -> None: ...
@overload
def __init__(self, x: Union[str, bytes], base: int = 10) -> None: ...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All defaults should be ....

@sproshev

Copy link
Copy Markdown
Contributor Author

We have the issue https://youtrack.jetbrains.com/issue/PY-23540 where we raise a warning on return str.__new__( cls, *args, **kwargs ) line.

@sproshev
sproshev force-pushed the sproshev/str_int_new branch from cae181d to 565f07c Compare May 25, 2017 17:52
@sproshev

Copy link
Copy Markdown
Contributor Author

Not sure how to fix compatibility issue inside IntEnum :(

@JelleZijlstra

Copy link
Copy Markdown
Member

Thanks, that makes sense. mypy also gives a spurious error on calling int.__new__(int, '3', 10).

I don't have a great solution for the test failures either. We could # type: ignore them away, but I think mypy's parser won't accept that.

…nd `str`.

Add `__new__` to `enum.IntEnum` to override inherited `__new__`.
@sproshev
sproshev force-pushed the sproshev/str_int_new branch from 565f07c to c094dc6 Compare May 26, 2017 13:13
@sproshev
sproshev force-pushed the sproshev/str_int_new branch from c094dc6 to 08b2ac4 Compare May 26, 2017 15:40
@sproshev

Copy link
Copy Markdown
Contributor Author

Fixed!

@JelleZijlstra

Copy link
Copy Markdown
Member

Yes, this seems to work. I tested it on my codebase too and got no errors. I'll leave this open a few more days in case other maintainers have concerns, then merge it.

@gvanrossum

Copy link
Copy Markdown
Member

Hm, mypy on the issue from the PyCharm tracker reports this, both with and without the change:

__tmp__.py:19: error: Inconsistent metaclass structure for 'MyString'
__tmp__.py:24: error: Name 'spam' is not defined
__tmp__.py:24: error: "MyString" expects no type arguments, but 1 given

Other than that, no much difference. (Though I wonder what would happen if someone were to add SupportsInt as a base class to str, or when PEP 544 (duck typing) is introduced.

@JelleZijlstra
JelleZijlstra merged commit fed4e03 into python:master Jun 13, 2017
@sproshev
sproshev deleted the sproshev/str_int_new branch June 13, 2017 16:17
JelleZijlstra added a commit to JelleZijlstra/typeshed that referenced this pull request Jul 5, 2017
matthiaskramm pushed a commit that referenced this pull request Jul 6, 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.

3 participants