GV

GeezerVison

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

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Add __orig_bases__ to Generic by hmc-cs-mdrissi · Pull Request #7827 · python/typeshed

Conversation

@hmc-cs-mdrissi

@hmc-cs-mdrissi hmc-cs-mdrissi commented May 11, 2022

Copy link
Copy Markdown
Contributor

Resolves #7811

I'm unsure why Generic is a _SpecialForm when at runtime it's normal class. I tried to make smallest change to Generic that just adds __orig_bases__.

Sorry, something went wrong.

Uh oh!

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

All reactions
Mehdi Drissi
Add __orig_bases__ to Generic
88145e7
@github-actions

This comment has been minimized.

Sign in to view
Mehdi Drissi added 2 commits May 10, 2022 21:48
Mehdi Drissi
Merge branch 'master' into md/add_orig_bases
316e2ba
Mehdi Drissi
Remove final decorator for special form
4faf48f
@JelleZijlstra

JelleZijlstra commented May 11, 2022

Copy link
Copy Markdown
Member

Not sure this will make a difference to any type checkers. Generic is not treated like a regular base class.

All reactions

Sorry, something went wrong.

Uh oh!

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

@github-actions

github-actions Bot commented May 11, 2022

Copy link
Copy Markdown
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

All reactions

Sorry, something went wrong.

Uh oh!

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

@hmc-cs-mdrissi

hmc-cs-mdrissi commented May 11, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

Hmm, experimenting with pyright I don't see a difference locally with this change. If I try swapping to class and doing

class Generic:
  __orig_bases__: ClassVar[tuple[type, ...]]

It does look like pyright uses it. But downside is it causes other aspects of generic to no longer work.

Alternative that's safer for type checkers is add __orig_bases__ to class type. It's a lie for non generics but would fix some false positives. If that's an acceptable lie I can make that change.

All reactions

Sorry, something went wrong.

Uh oh!

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

@AlexWaygood

AlexWaygood commented May 11, 2022

Copy link
Copy Markdown
Member

Alternative that's safer for type checkers is add __orig_bases__ to class type.

I'm fine with this if we can figure out a way to add it to Generic, but I would be -1 on adding it to type. This attribute is basically an undocumented implementation detail (even the throwaway reference in PEP 560 explicitly warns you against using it). If we add it to type, that doesn't match the runtime, and means it'll start showing up in autocompletions in IDEs for all class objects. All that for a use case that seems fairly niche :/

srittau and hauntsaninja reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

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

@hmc-cs-mdrissi

hmc-cs-mdrissi commented May 11, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

I'm unsure how to make Generic stub declare this. It looks like each type checker special cases generic in ways that makes me unclear if it's possible to make them happy. Alternative approach I experimented with was,

class Generic:
    def __class_getitem__(cls, params: Any) -> Any: ...
    __orig_bases__: tuple[type, ...]

This approach breaks pyright's type variable binding logic. It does make __orig_bases__ recognized, but it's treated as tuple and all generics end up losing there type arguments. mypy test cases pass, but it doesn't seem to affect mypy.

I also tested adding it to _SpecialForm. That looks to be a noop and does nothing. The current approach of _GenericSpecialForm also looks to be a noop.

Amusingly deleting Generic entirely does affect type checkers and causes mypy to crash (pyright runs, with many thousands of errors).

My guess is to add it to Generic will require each type checker to add some logic for this special case as they likely all have non standard handling for Generic as a type stub. Unsure it's worthwhile doing a feature request to each type checker to handle Generic type stub.

I do agree this use case is pretty niche though and my guess is if type lie isn't worth it is probably better to close pr.

edit: Well I'll mention it to pyright and maybe take a look at mypy eventually if it can be small contribution there.

All reactions

Sorry, something went wrong.

Uh oh!

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

@hmc-cs-mdrissi hmc-cs-mdrissi mentioned this pull request May 11, 2022
Support type inference for __orig_bases__ as defined in PEP 560 microsoft/pyright#3442
Closed
@hauntsaninja

hauntsaninja commented May 11, 2022

Copy link
Copy Markdown
Collaborator

In general, trying to type various aspects of the runtime implementation of typing has been notoriously fragile with little benefit.

  • Type checkers tend to special case these constructs heavily, so you're likely to break a type checker and often unlikely to convince them that things work the way you typed it
  • Historically the runtime implementations have been very unstable, so it's just been busy work
  • Users interested in this kind of thing are often doing things that aren't expressible in the type system anyway

So yeah, nice if you could make it work for cheap, but seems like that's not the case :-) Maybe we should put a comment warning in the file.

AlexWaygood reacted with thumbs up emoji huntzhan reacted with confused emoji
All reactions
  • 👍 1 reaction
  • 😕 1 reaction

Sorry, something went wrong.

Uh oh!

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

@hauntsaninja hauntsaninja closed this May 11, 2022
@hmc-cs-mdrissi
hmc-cs-mdrissi deleted the md/add_orig_bases branch May 31, 2022 00:23
@AlexWaygood AlexWaygood mentioned this pull request Feb 8, 2023
Add typing.get_orig_bases and typing.get_orig_class python/cpython#101688
Closed
@AlexWaygood AlexWaygood mentioned this pull request Apr 12, 2023
gh-101688: Implement types.get_original_bases python/cpython#101827
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

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

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support type.__orig_bases__ as defined in PEP 560

Uh oh!

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

4 participants

@hmc-cs-mdrissi @JelleZijlstra @AlexWaygood @hauntsaninja
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.