Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A simple start towards an implementation of import pygame.debug #3148

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

MyreMylar
Copy link
Member

@MyreMylar MyreMylar commented Oct 6, 2024

Was reading through PR #2944 and after seeing 43 files changed and looking over the original issue #2894 I thought perhaps it would be better to start with a smaller PR before building up to bigger systemic changes to warnings.

This PR does three things:

  1. Adds a new type of python warning, custom to pygame PygameDebugWarning.
  2. Adds a python file called debug.py that imports pygame, prints the old support prompt and the output of .print_debug_info() and sets the python warning filter level to allow the new python warning type PygameDebugWarning.
  3. Removes the support prompt printing from import pygame and filters out all warnings of the new type PygameDebugWarning.

This has the benefit of being small, easier to comprehend and an easier place to start a point of discussion on the main topics that might be contentious.

Test programs:

  1. Produces nothing in the console.
# without debug information - "normal mode"
import pygame

pygame.init()
  1. Produces 18 lines of output.
# with debug information - "debug mode"
import pygame.debug

pygame.init()

Output of 2.

pygame-ce 2.5.2.dev1 (SDL 2.30.7, Python 3.11.1)
Platform:		Windows-10-10.0.22631-SP0
System:			Windows
System Version:		10.0.22631
Processor:		AMD64 Family 23 Model 113 Stepping 0, AuthenticAMD	SSE2: Yes	AVX2: Yes	NEON: No
Architecture:		Bits: 64bit	Linkage: WindowsPE

Python:			CPython 3.11.1 (tags/v3.11.1:a7a450f, Dec  6 2022, 19:58:39) [MSC v.1934 64 bit (AMD64)]
pygame version:		2.5.2.dev1
SDL versions:		Linked: 2.30.7	Compiled: 2.30.7
SDL Mixer versions:	Linked: 2.8.0	Compiled: 2.8.0
SDL Font versions:	Linked: 2.22.0	Compiled: 2.22.0
SDL Image versions:	Linked: 2.8.2	Compiled: 2.8.2
Freetype versions:	Linked: 2.11.1	Compiled: 2.11.1

Display Driver:		Display Not Initialized
Mixer Driver:		Mixer Not Initialized

This could grow on from here adding any more esoteric/opinionated/potentially unwise usage debug warning information to the new category type.

@MyreMylar MyreMylar requested a review from a team as a code owner October 6, 2024 16:10
src_py/__init__.py Outdated Show resolved Hide resolved
@MyreMylar
Copy link
Member Author

Tinkered around with supressing deprecation warnings as part of this - but it is difficult to do in a sensible way as you can only supress python warnings by where the offending code originates, and deprecation warnings come, generally, from users using deprecated code.

@MyreMylar
Copy link
Member Author

MyreMylar commented Oct 6, 2024

What we might be able to do fairly easily is add a specific custom python warning category that we can then filter in, or filter out by default using PyErr_NewException("pygame.PygameDebugWarning", PyExc_Warning, NULL) without messing with warnings from other modules.

See an example here: https://github.com/danos/pygobject/blob/6372b569b1a9b6499ba73775350c8420017a1992/gi/gimodule.c#L684

@MyreMylar MyreMylar changed the title A simple implementation of import pygame.debug A simple start towards an implementation of import pygame.debug Oct 6, 2024
@MyreMylar
Copy link
Member Author

I added a new warning type that is only active when pygame.debug is imported. Currently nothing uses it.

@damusss
Copy link
Member

damusss commented Oct 6, 2024

not too sold on the name. pygame.error isn't named pygame.PygameError, perhaps this should be called pygame.DebugWarning?

@MyreMylar
Copy link
Member Author

MyreMylar commented Oct 6, 2024

not too sold on the name. pygame.error isn't named pygame.PygameError, perhaps this should be called pygame.DebugWarning?

I thought that initially, but only the second part on the string actually prints to the console if you create one, the first part is just the module name and in this case we actually want it to be clear it is a pygame only type of warning not a generic one that may collide or overlap with warnings from other modules.

Example from my testing:

circles_testing.py:8: PygameDebugWarning: pygame.Color.set_length deprecated since 2.1.3
  red_colour.set_length(2)

Copy link
Contributor

@gresm gresm left a comment

Choose a reason for hiding this comment

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

Besides minor notes, it would be nice if pygame.debug was documented and not a private, obscure submodule. You can just copy the changes from my PR (requires alteration), but otherwise I like this solution.

@@ -27,3 +27,4 @@ pygame\.pypm
pygame\._sdl2\.mixer
pygame\.sysfont.*
pygame\.docs.*
pygame\.debug
Copy link
Contributor

Choose a reason for hiding this comment

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

My guess that this change is here is that there's no debug.pyi stub, correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

As of currently, using:

import pygame.debug
pygame.do_stuff()

Works, as also:

import pygame.debug
import pygame as pg
from pygame import do_stuff

pg.do_stuff()
do_stuff()

But not:

import pygame.debug as pg
from pygame.debug import do_stuff

pg.do_stuff()
do_stuff()

What's your stance on this? If the last behavio(u)r is desired, adding

from pygame import *

Or

def __getattr__(name):
    return getattr(pygame, name)

def __dir__():
    return dir(pygame)

Should suffice, but if doing so, the debug.pyi stub should be created (edit buildconfig/stubs/gen_stubs.py to copy __init__.pyi stub).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants