Add colordict and sysfont to typing, move PowerState class to typing module

[ankith26]

• colordict and sysfont are actually public modules, so they are now typed.
• PowerState class has now been moved to typing.py, so that it can be resolved at the stub level correctly. But it is a private member of typing.py.

[damusss]

How can PowerState be private if it's not prefixed with an underscore? (not documenting it isn't really enough when it comes to python files)

[ankith26]

When I say private, I mean that it's not a part of __all__. I know it will be accessible from the typing module, but that will still be the case if it is prefixed with an underscore. Eventually this could be one of the things we add to __all__, I just want to keep API decisions out of this PR to keep it simpler to review

[damusss]

Alright then, LGTM :)

[Starbuck5]

Even if this is more accurate I don't want to leak the internal implementation details into our stable "type API"

For example, we could reduce memory usage or do interesting things with THECOLORS, right? Like what if we had it as a str to one integer (RRGGBBAA) instead of (R, G, B, A). We could go from the overhead of 4 python integers and a tuple down to 1 integer, which would be a bit less memory. No, we can't mess with it because it always has to be a dictionary of str to RGBA, and is exposed as public API by the type system.

(This is something I ran into the other day, before or after your PR the THECOLORS has already been leaked. I'm using it as an example of over-typing restricting our freedom to change things)

With that in mind I don't see why we would expose the existence of the colordict and sysfont modules, given they are 100% implementation details.

I'm also somewhat wary of moving an actual full declaration of one our classes into typing.py, that seems strange.

[ankith26]

Well, if they were intended to being implementation details but got leaked at runtime, they are not any more.

A quick github search reveals COLORDICT is already being used in user code, and it's mostly code it is probably older than all of our typestubs! It is also documented at https://pyga.me/docs/ref/color_list.html . Even this documentation is probably older than our typestubs.

pygame.sysfont is also "auto-imported" at runtime. As expected, a quick github code search will show that this is also used in user code. Even though at the moment it is explicitly ignored in the stubs.

My conclusion? Hiding or not hiding something in the stubs does not change the fact that implementation behaviour that is exposed eventually becomes API. And because we are now not at the liberty to remove it, we might as well roll with it and make it solid

[ankith26]

I'm also somewhat wary of moving an actual full declaration of one our classes into typing.py, that seems strange.

It seemed weird to have a singleton module just for defining a class. I feel like it logically belongs to typing.py

[aatle]

I feel like it logically belongs to typing.py

I disagree.
The typing module is only for classes that exist for the benefit of static typing. This does not include misc concrete classes. typing.py. (In fact, there are currently no concrete classes in pygame.typing, for good reasons.)
Consider that PowerState has no actual connection or relation to typing (not any more than any of the other pygame classes).

Yes, PowerState being a single class with its own module is strange, and a change would be appropriate. But moving it to typing.py as if the module were a utility module is not the solution.

[ankith26]

Well, even if we have to keep it as it is, I would still like to give it the typing treatment for consistency reasons (that is, duplicating _data_classes.py to _data_classes.pyi). It's one of the things mypy seems to occasionally complain about.

[aatle]

Would it be possible to type PowerState in system.pyi instead of _data_classes.pyi, or would this be worse?

[ankith26]

That is also doable, but then we have to maintain the duplication of the definition in the source and the stub manually.

[Starbuck5]

I don't think sysfont is supposed to be auto-imported. I think the authors of this primordial code imported it so the could put functions defined in that file onto the font module, and then they neglected the "del sysfont" step to remove it from the namespace.

Just because an implementation detail is leaked doesn't mean it should be treated as an API. Where are the docs for the sysfont module? Where are the examples of use? They aren't there and they shouldn't be there. There are tests but that makes sense as as a test of a system.

As far as I'm concerned in pygame-ce 3 we should rename sysfont to _ankith26_is_a_cool_guy_and_great_at_ci_stuff.py.

Similarly, colordict is not a module, it's an implementation detail. That's why if you go to the top of the docs site there isn't a place called "colordict".

>>> pygame.colordict
<module 'pygame.colordict' from 'C:\\Users\\charl\\AppData\\Local\\Programs\\Python\\Python39\\lib\\site-packages\\pygame\\colordict.py'>

Ok fine it's a module at runtime but there's no reason it should or has to be, and I don't think we should treat it how we treat something like font or draw.

[Starbuck5]

And in terms of powerstate can the pyi file import a definition from a py file?

[ankith26]

And in terms of powerstate can the pyi file import a definition from a py file?

It can, but sometimes it doesn't and then mypy complains about it. I don't know how or why it happens though

[Starbuck5]

If we had a pure Python module would we need type stubs for it at all?

What if pygame.system was a python module that called into C code to provide its functionality?

[ankith26]

If we had a pure Python module would we need type stubs for it at all?

Technically no, but so far we have had stubs for the python modules as well. One of the things here that I think is causing mypy analysis issues is that the stubs and python sources are in different folders when they should be in the same folder (and after installation, they actually are)