Add runtime support for pygame.sprite.AbstractGroup subscripts

[Matiiss]

Currently this code would fail at runtime

import pygame


class MySprite(pygame.sprite.Sprite):
    ...


my_group: pygame.sprite.Group[MySprite] = pygame.sprite.Group()

with

Traceback (most recent call last):
  File "C:\Users\Matiiss\PycharmProjects\Something\for_so.py", line 8, in <module>
    my_group: pygame.sprite.Group[MySprite] = pygame.sprite.Group()
              ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^
TypeError: type 'Group' is not subscriptable

Currently a viable alternative is to use

my_group: "pygame.sprite.Group[MySprite]" = pygame.sprite.Group()

or

from __future__ import annotations

However, I feel as though that makes it more cumbersome for the end user.
Also this solves a cross compatibility issue with pg/pg

Besides this, the type stubs for pygame.sprite have to be massively revised, they are quite flawed it seems, especially when this Generic mechanism is taken into account, but I will handle that in a another PR.

[Starbuck5]

Why is this imported here and not the top of the file?

[Matiiss]

Because I did not want to introduce typing in the global namespace of this module, I'm not sure why typing is imported here at all, that should be fixed by removing all those type hints, for one, they are meaningless in this file (unless someone decides on doing runtime annotation inspection, but if we want to support that, we'd have to move all the type hints from stub files to source code)

So, in essence, this module is used in this one method, that is unlikely to be used by many in the first place and when it is used, it wouldn't really hurt performance or anything. In other words, I don't see a reason why it couldn't be imported in this method and be sort of self-contained.

[Starbuck5]

If you don't want typing in the global namespace of the module, then you could del it at the bottom.

Also it's kind of an ironic take that type hints in source code are useless when promoting the adoption and support of type hints for users of said source code. Even though I know what you're mean, and you're right.

It still irks me that stubs mean we can't let our Python source shine through. Could the stub import the runtime?

[Matiiss]

If you don't want typing in the global namespace of the module, then you could del it at the bottom.

That is unfortunately not possible because it will fail as runtime when __class_getitem__ is called because it won't be able to find the name typing anymore.

Could the stub import the runtime?

If you mean whether the stub file can import this file, the answer is yes (AFAIU). However, annotating the code in the source might be a bit inconsistent.

[Starbuck5]

So _GenericAlias isn't just a hidden version of GenericAlias on 3.8, it seems like a subtly different thing:
https://stackoverflow.com/questions/74412803/what-is-typing-genericalias-in-python-typing

Maybe we should conditionally define this whole function for 3.9 and above, and use GenericAlias?

But when I look up how to make user defined generic classes, I get a different result entirely: https://typing.readthedocs.io/en/latest/guides/modernizing.html#user-defined-generics . Inherit from generic and use type vars?

[Matiiss]

A conditional might be an approach, though it shouldn't really matter (whether we use a conditional or wait until 3.8 support is dropped and just replace it). Thing is, type checkers don't care what is in this file, they look only at the stubs, so I think that in favor of consistency, there shouldn't be any type annotations in here. Besides, these annotations already exist in the type stubs, we would be repeating code by introducing TypeVars and Generics here.

On top of that, from a semantic perspective it wouldn't make sense because we would create a single TypeVar just to be able to use this Generic inheritance and then that TypeVar won't be used anywhere ever again in this file because if you start typing the rest of it using T as the return type of various methods, that's just literally repeating what is already in the type stubs, except worse and to no benefit to anyone.

So I find it easier/better/clearer to simply support __class_getitem__ at runtime, it could just return cls, but returning this _GenericAlias allows some more runtime interfacing regarding a couple typing functions (that the tests cover)

[Starbuck5]

Thing is, type checkers don't care what is in this file, they look only at the stubs, so I think that in favor of consistency, there shouldn't be any type annotations in here.

If type checkers don't care why are you returning anything at all? Just return None and then I won't be concerned about you using an undocumented part of the typing module.

So I find it easier/better/clearer to simply support class_getitem at runtime, it could just return cls, but returning this _GenericAlias allows some more runtime interfacing regarding a couple typing functions (that the tests cover)

Ah you've addressed that already, okay.

Anyway my main problem with this PR is you're using an undocumented thing that barely exists on the internet and will need to be replaced later, and that the undocumented thing is different than what it allegedly needs to be replaced with!

On another note, the community of people who are clamoring for advanced type hinting features but are still on 3.8 has got to be pretty small/nonexistent. So if it's possible to conditionally define this function and do it more correctly only supporting higher versions I'd like to pursue that.

[Matiiss]

Fine, will do a conditional, I'll also move the import out of this method and to the top of the file:

import sys

if sys.version >= (3, 9, 0):
    from types import GenericAlias
else:
    from typing import _GenericAlias as GenericAlias  # type: ignore[name-defined]

[damusss]

LGTM, with this change one doesn't have to wrap the subscript inside a string for annotations.

I don't think importing typing inside the function is a problem because:

• The class is subscripted a little amount of times
• Sequential subscripts will use the cached module, so it's just like if you import it at the top of the file

Regarding the generic implementation, I think the sprite stubs do a good enough job with subclassing Generic and using the typevars, so I think the actual implementation can ignore all that and only implement class getitem for the runtime.

Thanks :)