Fix and improve pygame.typing module

[aatle]

Fixes and changes from #3074 and more. (Addresses various typing problems about correctness.)

This draft includes all of my proposed fixes, though some of them may need more discussion.
I wasn't able to fully test with mypy because of my current setup.

Also, perhaps an __all__ list can be created for this typing module?

[damusss]

To use mypy you can pip install it and run the file inside buildconfig called stubcheck.py. remember to install your branch locally before doing so. I will do that for this changes on my machine too

[ankith26]

The mypy on CI is failing with the following error

typing_sample_app.py:114: error: Argument 1 to "validator_RectLike" has incompatible type "list[Collection[int | float]]"; expected "SequenceLike[float] | SequenceLike[SequenceLike[float]] | _HasRectAttribute"  [arg-type]
Found 1 error in 1 file (checked 1 source file)

[aatle]

I think I fixed the error (caused by bad type inference).
I also added an __all__ list to better distinguish exported types from imports.

I'd like to hear your thoughts on two possible removals:

Remove _CanBeRect type. It is currently internal, only used once, and can easily be combined with RectLike, which is also a Union. (Plus, the name isn't great.)

Remove `PathLike` union type

• I don't think it is appropriate for pygame to define such a general-usage type, as it isn't very related to pygame. (FileLike, though, is mostly pygame-specific.)
  • For example, the python typeshed already defines an equivalent StrOrBytesPath as an internal type.
• Currently used only once, in pygame.encode_string().
• PEP 519 considered adding this same union to the standard library, but did not.
• Easier to add the class in than to remove it, in the future.

@damusss, one question, what is the reasoning behind _PathProtocol being defined for python 3.8 instead of imported?

[Starbuck5]

So this PR brings up a question for me.

Do we need type stub files for python modules, or can we just put type information into python source for those modules?

Type checkers should merge the stub package and runtime package or typeshed directories. This can be thought of as the functional equivalent of copying the stub package into the same directory as the corresponding runtime package or typeshed folder and type checking the combined directory structure. Thus type checkers MUST maintain the normal resolution order of checking *.pyi before *.py files.

This part of https://peps.python.org/pep-0561/ makes me think no. Rather than a typing.pyi we can put it with the runtime? And @Matiiss can make sprite groups (#3053) typed in the normal way in the python source instead of needing to worry without the difference between source and stubs?

[Starbuck5]

Although @zoldalma999 is working on putting docstrings into stubs (or something like that), so maybe it's advantageous to keep them.

[ankith26]

I think your older commit that added the [(3, 3.2), pygame.Vector2()] testcase was technically acceptable. If the typechecker is rejecting it, then I believe there is a bug somewhere

[ankith26]

Remove _CanBeRect type. It is currently internal, only used once, and can easily be combined with RectLike, which is also a Union. (Plus, the name isn't great.)

I agree, this change should not have user facing consequences I believe

[damusss]

@aatle PathLike didn't exist in the os library before python 3.9, that's the reason

[Matiiss]

So this PR brings up a question for me.

Do we need type stub files for python modules, or can we just put type information into python source for those modules?

Type checkers should merge the stub package and runtime package or typeshed directories. This can be thought of as the functional equivalent of copying the stub package into the same directory as the corresponding runtime package or typeshed folder and type checking the combined directory structure. Thus type checkers MUST maintain the normal resolution order of checking *.pyi before *.py files.

This part of https://peps.python.org/pep-0561/ makes me think no. Rather than a typing.pyi we can put it with the runtime? And @Matiiss can make sprite groups (#3053) typed in the normal way in the python source instead of needing to worry without the difference between source and stubs?

I can side with this, but note that it would require quite a bit of effort to move all Python source typing from the stubs to the source code, but I'm not against it. However, it does mean that all the protocol stuff will have to be implemented in the source code as well, which I'm not against either and it will help expose all these internal types to the end user, but it's just something to be noted. This also means that pygame.typing will be left mostly with type hints that do not exist in the Python code after this move.

[damusss]

Remove PathLike union type

• I don't think it is appropriate for pygame to define such a general-usage type, as it isn't very related to pygame. (FileLike, though, is mostly pygame-specific.)

  • For example, the python typeshed already defines an equivalent StrOrBytesPath as an internal type.

• Currently used only once, in pygame.encode_string().

• PEP 519 considered adding this same union to the standard library, but did not.

• Easier to add the class in than to remove it, in the future.

I checked and it's only used inside rwobject.pyi, so I guess you can add the current value of PathLike inside FileLike and define PathLike internally just for rwobject.pyi only (prefixed by an underscore, similar to how formats are annotated for pygame.image). You might want the opinion of the steering council aswell on this one, since PathLike depends on os.PathLike which is not available before 3.9, and _typeshed is not a valid module at runtime.

[aatle]

@Starbuck5

Do we need type stub files for python modules, or can we just put type information into python source for those modules?

Type checkers should merge the stub package and runtime package or typeshed directories. This can be thought of as the functional equivalent of copying the stub package into the same directory as the corresponding runtime package or typeshed folder and type checking the combined directory structure. Thus type checkers MUST maintain the normal resolution order of checking *.pyi before *.py files.

This part of https://peps.python.org/pep-0561/ makes me think no. Rather than a typing.pyi we can put it with the runtime?

There are a few reasons why it is preferable to have separate .pyi stub files for pygame's situation.

1. C modules cannot be type annotated in their source. Many of the modules in pygame are written in C, and thus require separate stub files for type information. For python modules, it may be desirable to also use stub files for consistency.
2. Separate typehinting from actual implementation. It may be easier to maintain them separately than together. Type hinting requires much more maintenance and thought, and a single programmer may not want to deal with that when writing new python code or browsing the blame. Also, less clutter, especially from imports, in source files.

About your quote from PEP 561, I don't think it is relevant to the question. It refers to what type checkers like mypy should do, not programmers. It does not say to combine the stub files with source files; rather, it says to combine the stub directories with source directories. (Equivalent to putting all of the *.pyi files in same directory as their corresponding source code.)

[aatle]

@ankith26

I think your older commit that added the [(3, 3.2), pygame.Vector2()] testcase was technically acceptable. If the typechecker is rejecting it, then I believe there is a bug somewhere

The error was a result of normal assumptions and inferences by mypy. Mypy, when seeing an unannotated list with multiple types inside, will find a common base class to use (not a union). In this case, it chooses Collection, certainly valid but errors for the situation. Mypy can't use Sequence and wouldn't know to use SequenceLike.
If the list were instead explicitly annotated with a union or SequenceLike[float] inside, or even if either of the list items were explicitly annotated as a SequenceLike, there would've be no error for that test case.

[aatle]

@damusss I must be missing something, wasn't os.PathLike added in python 3.6?

[damusss]

@aatle my bad, what I said was BS, but there's still an issue before 3.9:

failed checks:

Basically before 3.9 you can't do os.PathLike[str] or os.PathLike[bytes]
sorry for the confusion

[aatle]

Ahh, okay. I had actually found that reason before but forgot and couldn't remember.
Anyways, do you think it would be any better to still use os.PathLike instead of defining our own, before 3.9:

from os import PathLike as _PathProtocol

if sys.version_info >= (3, 9):
    PathLike = Union[str, bytes, _PathProtocol[str], _PathProtocol[bytes]]
else:
    # os.PathLike is not generic in python 3.8
    PathLike = Union[str, bytes, _PathProtocol]

[damusss]

@aatle I don't think so. Without specifying str or bytes, one could make this code and it would pass, which is obviously wrong:

class MyPath:
    def __fspath__(self) -> int: return 0
    
pygame.rwobject.encode_string(MyPath()) # or whatever function that needs PathLike

I checked and mypy passes.

[ankith26]

I like this solution you just posted.

The error was a result of normal assumptions and inferences by mypy. Mypy, when seeing an unannotated list with multiple types inside, will find a common base class to use (not a union). In this case, it chooses Collection, certainly valid but errors for the situation.

Yeah this makes sense, thanks for the explanation. Now I'm wondering if this can somehow be fixed by declaring SequenceLike to inherit from Collection.

[aatle]

@damusss os.PathLike has the same problem in 3.8 too, and it's very minor, right? Also, what typeshed version are you using for the 3.8 checks? The benefit of the change is using a standard type instead of custom one.

@ankith26 Probably not 'fixable', it is a small and easily fixable issue, and we shouldn't require __contains__ and __iter__ for SequenceLike.

[aatle]

Need more opinions on removal of typing.PathLike.
I think a single extra definition for rwobject.pyi is fine, if PathLike is removed.

[damusss]

@aatle what exactly do you mean with typeshed version?

Also I don't disagree with the removal of PathLike if the SC if ok with it.

[aatle]

@damusss, if I understand correctly, the standard library alone does not have type hints (such as for what the return value of __fspath__ is), the typeshed provides this information (often is bundled with type checker), I think. Like, I'm not sure if python 3.9 specifies the type either; in the source, it just defines __class_getitem__ without any bound or constrained types, and the typeshed is what you have to look for it in.

[damusss]

@aatle but what exactly is the typeshed? _typeshed.pyi does not exist at runtime

[aatle]

I mean https://github.com/python/typeshed, according to the info in the readme. E.g. https://github.com/python/typeshed/blob/main/stdlib/os/__init__.pyi

[ankith26]

I think for the purposes of this PR, we should retain PathLike, for the purposes of keeping this PR easier to merge. We can of course discuss its removal in a future PR or issue

[damusss]

@aatle I just realized you can probably import StrOrBytesPath from _typeshed by guarding it inside a typing.TYPE_CHECKING block. not sure it would work in 3.8 but it's worth a try.

[aatle]

I'm going to wrap up this PR for merging.
I may create an issue for removal of PathLike and changes to _PathProtocol, later, before the pygame.typing module is released in 2.5.2.

[damusss]

I agree to keep this PR as bugfix/improvement, and modify the API in other PRs.
I've looked at the code and tested locally, LGTM! 👍
Thank you for improving my beloved module :)

(do you think abstractmethod can be deleted at the end of the file? just wondering)

[aatle]

(do you think abstractmethod can be deleted at the end of the file? just wondering)

Yes, it can. I'll just add that in real quick.

[ankith26]

LGTM, thanks for contributing! 🥳