GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — bpo-40336: Refactor typing._SpecialForm by serhiy-storchaka · Pull Request #19620 · python/cpython

Conversation

serhiy-storchaka commented Apr 20, 2020 •

edited by bedevere-bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

https://bugs.python.org/issue40336

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          bpo-40336: Refactor typing._SpecialForm

443b246

serhiy-storchaka added the skip news label

Apr 20, 2020

serhiy-storchaka requested review from gvanrossum and ilevkivskyi as code owners

April 20, 2020 11:29

the-knights-who-say-ni added the CLA signed label

Apr 20, 2020

bedevere-bot added the awaiting core review label

Apr 20, 2020

ilevkivskyi approved these changes

Apr 20, 2020

View reviewed changes

ilevkivskyi left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks, just couple optional suggestions (up to you).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/typing.py

		isinstance(args[0], str) and
		isinstance(args[1], tuple)):
		# Close enough.
		raise TypeError(f"Cannot subclass {cls!r}")

ilevkivskyi Apr 20, 2020

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

It would be good to keep a custom error message, but not important.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

serhiy-storchaka Apr 23, 2020

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

You are right, I tested again class A(Any): pass, and got "__init__() takes 2 positional arguments but 4 were given" which does not look user friendly.

But the former message was "Cannot subclass <class 'typing._SpecialForm'>" which does not look correct, because we subclass an instance of _SpecialForm, not _SpecialForm itself. It is not possible to fix in __new__, but we can raise better error in custom __mro_entries__.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

ilevkivskyi Apr 23, 2020

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Using __mro_entries__ is a great idea!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/typing.py

		return self._name

		def __call__(self, args, *kwds):
		raise TypeError(f"Cannot instantiate {self!r}")

ilevkivskyi Apr 20, 2020

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Again, a custom exception message may be helpful, but not important.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

serhiy-storchaka Apr 23, 2020

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Your are right. "Cannot instantiate typing.Any" looks better than "'_SpecialForm' object is not callable". Initially I renamed _SpecialForm to special form, but later reverted this change.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

ilevkivskyi Apr 23, 2020

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Perfect! Thanks for fixing this.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/typing.py

		# There is no '_type_check' call because arguments to Literal[...] are
		# values, not types.
		return _GenericAlias(self, parameters)
		raise TypeError(f"{self} is not subscriptable")

ilevkivskyi Apr 20, 2020

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Splitting this is really great. I always wanted to do this, but didn't find a way to do this that wouldn't require boilerplate code, it looks like you have found such way.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bedevere-bot added awaiting merge and removed awaiting core review labels

Apr 20, 2020

gvanrossum commented Apr 20, 2020

Copy link

Copy Markdown

Member

I'll leave this to Ivan -- I am busy with the PEG parser integration.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Restore __call__ and add __mro_entries__ for better error messages.

4596cf2

ilevkivskyi approved these changes

Apr 23, 2020

View reviewed changes

serhiy-storchaka merged commit 40ded94 into python:master

Apr 23, 2020

bedevere-bot removed the awaiting merge label

Apr 23, 2020

serhiy-storchaka deleted the refactor-typing-special-form branch

April 23, 2020 18:26

colesbury referenced this pull request in colesbury/nogil

Oct 6, 2021


          bpo-40336: Refactor typing._SpecialForm (GH-19620)

a7590c3

AlexWaygood mentioned this pull request

Apr 17, 2023

Fix various things with Literal python/typing_extensions#145

Merged

Reviewers

ilevkivskyi

ilevkivskyi approved these changes

gvanrossum

Awaiting requested review from gvanrossum

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

serhiy-storchaka commented Apr 20, 2020 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ilevkivskyi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gvanrossum commented Apr 20, 2020

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

serhiy-storchaka commented Apr 20, 2020 •

edited by bedevere-bot

Loading

Uh oh!

There was an error while loading. Please reload this page.