Skip to content

bpo-28556: Minor fixes for typing module#6732

Merged
ilevkivskyi merged 4 commits into
python:masterfrom
ilevkivskyi:sync-typing
May 9, 2018
Merged

bpo-28556: Minor fixes for typing module#6732
ilevkivskyi merged 4 commits into
python:masterfrom
ilevkivskyi:sync-typing

Conversation

@ilevkivskyi

@ilevkivskyi ilevkivskyi commented May 8, 2018

Copy link
Copy Markdown
Member

@ilevkivskyi

Copy link
Copy Markdown
Member Author

Note this will not backport to 3.6 cleanly. I will make a manual backport.

@gvanrossum

gvanrossum commented May 8, 2018 via email

Copy link
Copy Markdown
Member

@ilevkivskyi

Copy link
Copy Markdown
Member Author

It looks like all failures are unrelated except for a useful warning about bad .rst formatting. Let us see tomorrow how it goes.

@ilevkivskyi

Copy link
Copy Markdown
Member Author

Hm, all tests passed now. So either they were flakes, or just they got fixed in the meantime. I think it is safe to merge now.

@ilevkivskyi
ilevkivskyi merged commit 43d12a6 into python:master May 9, 2018
@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @ilevkivskyi for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@ilevkivskyi
ilevkivskyi deleted the sync-typing branch May 9, 2018 01:23
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 9, 2018
This also fixes https://bugs.python.org/issue33420
(cherry picked from commit 43d12a6)

Co-authored-by: Ivan Levkivskyi <levkivskyi@gmail.com>
@bedevere-bot

Copy link
Copy Markdown

GH-6735 is a backport of this pull request to the 3.7 branch.

miss-islington added a commit that referenced this pull request May 9, 2018
This also fixes https://bugs.python.org/issue33420
(cherry picked from commit 43d12a6)

Co-authored-by: Ivan Levkivskyi <levkivskyi@gmail.com>
Comment thread Lib/typing.py
"follow default field(s) {default_names}"
.format(field_name=field_name,
default_names=', '.join(defaults_dict.keys())))
nm_tpl.__new__.__annotations__ = collections.OrderedDict(types)

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.

Why OrderedDict? dict is ordered.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

To make code closer to the backported version (plus there is another occurrence above). At some point however we might want to clean-up all this, but I propose to make it a separate PR.

Comment thread Lib/typing.py
raise TypeError("Type Generic cannot be instantiated; "
"it can be used only as a base class")
return super().__new__(cls)
if super().__new__ is object.__new__:

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.

I think and cls.__init__ is not object.__init__ is needed here.

The test:

        class A(Generic[T]):
            pass
        with self.assertRaises(TypeError):
            A('foo')

And would extract the subexpression super().__new__ into a variable.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hm, indeed, in the original PR in typing repo we were concerned that it may fail when it shouldn't, but forgot that it also should fail when it should. @gvanrossum what do you think?

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.

Yes, @serhiy-storchaka is right. This is definitely a wart -- object.__init__ and object.__new__ each ignore extra arguments only when the other is overridden (and they themselves are not).

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.

6 participants