Skip to content

Ignore pygame deprecation warnings during testing #1284

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dm-ackerman
Copy link
Contributor

Description

pygame is using a deprecated packaging api. Since it has been raising warnings for years, it seems unlikely that the pygame devs are interested in fixing that any time soon. This change ignores those warnings during testing. If pygame devs ever see the light and correct this, these filters should be removed.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have run the pre-commit checks with pre-commit run --all-files (see CONTRIBUTING.md instructions to set it up)
  • I have run pytest -v and no errors are present.
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I solved any possible warnings that pytest -v has generated that are related to my code to the best of my knowledge.
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

pygame is using a deprecated packaging api. Since it has been raising warnings for years, it seems unlikely that the pygame devs are interested in fixing that any time soon. This change ignores those warnings during testing. If pygame devs ever see the light and correct this, these filters should be removed.
@David-GERARD David-GERARD self-requested a review April 18, 2025 14:42
@David-GERARD David-GERARD self-assigned this Apr 18, 2025
@David-GERARD David-GERARD added the enhancement New feature or request label Apr 18, 2025
@David-GERARD David-GERARD added this to the 1.25.1 milestone Apr 18, 2025
@David-GERARD
Copy link
Collaborator

@dm-ackerman is the warning really an issue? What is improved by ignoring it?

@dm-ackerman
Copy link
Contributor Author

Several reasons:

  • The warning isn't useful as we can't do anything about it.
  • We ask people submitting PRs to resolve warnings created by their changes, but the base code throws a ton of warnings (about 200 on my system). That sends the message that we don't actually care if people fix warnings, which encourages them not to, which leads to more warnings.
  • It normalizes deviance by making warnings OK, which reduces the value of warnings by encouraging people to ignore them. Warnings become a flood of meaningless noise rather than actionable items to look at.
  • It makes it less obvious when changes cause a warning (3 warnings when there used to be 0 is obvious. The current number varies with the number of processes, so it's not always obvious that the number has increased).

I concede that none of these rises to the level of major problem, but the change to fix it isn't major either. It's a very targeted filtering of a specific type of warning from two specific sources.

Is there a reason not to do this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants