Skip to content

Conversation

ankith26
Copy link
Member

@ankith26 ankith26 commented Oct 8, 2025

The mypy update is needed for full 3.14 support, old mypy was raising false positive issues.

With the update, I had to make fixes for the new issues mypy raises.

Finally, I also took the opportunity to update pre-commit/pylint.

@ankith26 ankith26 requested a review from a team as a code owner October 8, 2025 04:25
Copy link
Contributor

coderabbitai bot commented Oct 8, 2025

📝 Walkthrough

Walkthrough

Tooling and CI version bumps; gen_stubs.py adds a fmt_list helper and uses it to emit all entries. Many pygame stub modules gain explicit all lists. Several stub classes change from init signatures to new. stubtest gets a new flag and minor C comment formatting was adjusted.

Changes

Cohort / File(s) Summary
Tooling / CI
\.pre-commit-config.yaml, dev.py, .github/workflows/build-ubuntu-sdist.yml
Bump pre-commit hooks and dev tool versions; workflow no longer pins numpy and runs python3 dev.py stubs for typestub checks.
Stub generation helper
buildconfig/stubs/gen_stubs.py
Add fmt_list(seq: list[str]) and use it to write __all__ for generated stubs (constants, locals).
Module __all__ additions
buildconfig/stubs/pygame/constants.pyi, .../locals.pyi, .../freetype.pyi, .../midi.pyi, .../sndarray.pyi, .../surfarray.pyi, .../version.pyi
Add explicit __all__ lists enumerating public symbols (constants and API names); constants.pyi contains duplicated __all__ assignments.
Constructor → new in stubs
buildconfig/stubs/pygame/bufferproxy.pyi, .../joystick.pyi, .../pixelarray.pyi
Replace __init__(...) -> None signatures with __new__(cls, ...) -> <Class> in several stub classes.
Stubtest invocation formatting
buildconfig/stubs/stubcheck.py
Expand the stubtest argument list across multiple lines and add --ignore-disjoint-bases and explicit --allowlist usage.
Minor C formatting
src_c/SDL_gfx/SDL_gfxPrimitives.c
Whitespace/indentation changes inside a comment block only; no behavioral changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.54% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title directly references the primary changes in this pull request, which involve upgrading mypy and pre-commit and resolving the issues introduced by those updates. It succinctly captures the core purpose without extraneous details or vague wording. As a result, it clearly communicates the main intent and aligns with the changes in the diff.
Description Check ✅ Passed The description clearly states the rationale for updating mypy to support Python 3.14 and outlines that fixes were made for new issues. It also notes the update of pre-commit and pylint versions, matching the dependency updates in the diff. This content is directly related to the changes and provides relevant context.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ankith26-3.14-typing

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1c20063 and a3ec9ac.

📒 Files selected for processing (16)
  • .github/workflows/build-ubuntu-sdist.yml (2 hunks)
  • .pre-commit-config.yaml (3 hunks)
  • buildconfig/stubs/gen_stubs.py (3 hunks)
  • buildconfig/stubs/pygame/bufferproxy.pyi (1 hunks)
  • buildconfig/stubs/pygame/constants.pyi (1 hunks)
  • buildconfig/stubs/pygame/freetype.pyi (1 hunks)
  • buildconfig/stubs/pygame/joystick.pyi (1 hunks)
  • buildconfig/stubs/pygame/locals.pyi (1 hunks)
  • buildconfig/stubs/pygame/midi.pyi (1 hunks)
  • buildconfig/stubs/pygame/pixelarray.pyi (1 hunks)
  • buildconfig/stubs/pygame/sndarray.pyi (1 hunks)
  • buildconfig/stubs/pygame/surfarray.pyi (1 hunks)
  • buildconfig/stubs/pygame/version.pyi (1 hunks)
  • buildconfig/stubs/stubcheck.py (1 hunks)
  • dev.py (1 hunks)
  • src_c/SDL_gfx/SDL_gfxPrimitives.c (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
  • src_c/SDL_gfx/SDL_gfxPrimitives.c
  • dev.py
  • buildconfig/stubs/gen_stubs.py
  • .pre-commit-config.yaml
  • buildconfig/stubs/pygame/surfarray.pyi
  • buildconfig/stubs/pygame/constants.pyi
  • buildconfig/stubs/pygame/midi.pyi
  • buildconfig/stubs/pygame/freetype.pyi
  • .github/workflows/build-ubuntu-sdist.yml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-08T08:15:01.390Z
Learnt from: ankith26
PR: pygame-community/pygame-ce#3602
File: buildconfig/stubs/pygame/locals.pyi:601-1198
Timestamp: 2025-10-08T08:15:01.390Z
Learning: The repository pygame-community/pygame-ce is for pygame-ce (pygame community edition), not the original pygame project. Always refer to it as pygame-ce in reviews and analyses.

Applied to files:

  • buildconfig/stubs/pygame/locals.pyi
🧬 Code graph analysis (1)
buildconfig/stubs/pygame/pixelarray.pyi (2)
src_py/__init__.py (2)
  • Surface (227-228)
  • PixelArray (253-254)
buildconfig/stubs/pygame/surface.pyi (1)
  • Surface (42-1051)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: build (ubuntu-22.04)
  • GitHub Check: debug_coverage (ubuntu-24.04, 3.14.0rc1)
  • GitHub Check: debug_coverage (ubuntu-24.04, 3.9.23)
  • GitHub Check: x86_64
  • GitHub Check: dev-check
  • GitHub Check: x86
🔇 Additional comments (2)
buildconfig/stubs/pygame/sndarray.pyi (1)

14-23: LGTM! Good addition of explicit exports.

The __all__ declaration properly lists all public functions defined in this stub, improving export control. The sync comment is a helpful maintenance reminder.

buildconfig/stubs/pygame/pixelarray.pyi (1)

36-36: LGTM! Constructor signature correctly updated.

The change from __init__ to __new__ accurately reflects how C extension types construct instances. The explicit PixelArray return type improves type-checking precision, aligning with the PR's goal of resolving mypy issues.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ankith26 ankith26 force-pushed the ankith26-3.14-typing branch from 3a6839f to 1c20063 Compare October 8, 2025 07:41
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3a6839f and 1c20063.

📒 Files selected for processing (16)
  • .github/workflows/build-ubuntu-sdist.yml (1 hunks)
  • .pre-commit-config.yaml (3 hunks)
  • buildconfig/stubs/gen_stubs.py (3 hunks)
  • buildconfig/stubs/pygame/bufferproxy.pyi (1 hunks)
  • buildconfig/stubs/pygame/constants.pyi (1 hunks)
  • buildconfig/stubs/pygame/freetype.pyi (1 hunks)
  • buildconfig/stubs/pygame/joystick.pyi (1 hunks)
  • buildconfig/stubs/pygame/locals.pyi (1 hunks)
  • buildconfig/stubs/pygame/midi.pyi (1 hunks)
  • buildconfig/stubs/pygame/pixelarray.pyi (1 hunks)
  • buildconfig/stubs/pygame/sndarray.pyi (1 hunks)
  • buildconfig/stubs/pygame/surfarray.pyi (1 hunks)
  • buildconfig/stubs/pygame/version.pyi (1 hunks)
  • buildconfig/stubs/stubcheck.py (1 hunks)
  • dev.py (1 hunks)
  • src_c/SDL_gfx/SDL_gfxPrimitives.c (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • buildconfig/stubs/pygame/version.pyi
🚧 Files skipped from review as they are similar to previous changes (9)
  • buildconfig/stubs/gen_stubs.py
  • .pre-commit-config.yaml
  • buildconfig/stubs/stubcheck.py
  • buildconfig/stubs/pygame/constants.pyi
  • buildconfig/stubs/pygame/sndarray.pyi
  • buildconfig/stubs/pygame/bufferproxy.pyi
  • buildconfig/stubs/pygame/joystick.pyi
  • src_c/SDL_gfx/SDL_gfxPrimitives.c
  • buildconfig/stubs/pygame/freetype.pyi
🧰 Additional context used
🧬 Code graph analysis (1)
buildconfig/stubs/pygame/pixelarray.pyi (2)
src_py/__init__.py (2)
  • Surface (227-228)
  • PixelArray (253-254)
buildconfig/stubs/pygame/surface.pyi (1)
  • Surface (42-1051)
🪛 GitHub Actions: Ubuntu sdist
dev.py

[error] 1-1: Process exited with code 1 during stub generation: git shows uncommitted changes after running stubs (e.g., generated files '1.21.0' and 'sdlaudio.raw'). Commit the changes or run 'python3 dev.py stubs' followed by a commit, or run 'python3 dev.py all' to catch more issues.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
  • GitHub Check: x86_64
  • GitHub Check: arm64
  • GitHub Check: x86
  • GitHub Check: AMD64
  • GitHub Check: Debian (Bookworm - 12) [armv6]
  • GitHub Check: Debian (Bookworm - 12) [s390x]
  • GitHub Check: Debian (Bookworm - 12) [ppc64le]
  • GitHub Check: Debian (Bookworm - 12) [armv7]
  • GitHub Check: debug_coverage (ubuntu-24.04, 3.13.5)
  • GitHub Check: i686
🔇 Additional comments (3)
dev.py (1)

199-201: LGTM! Development dependency versions updated.

The version bumps align with the PR objective of updating mypy to 1.18.2 for Python 3.14 support, along with pylint and pre-commit updates.

.github/workflows/build-ubuntu-sdist.yml (1)

67-67: Workflow simplification approved.

Replacing the explicit mypy installation and stubcheck invocation with python3 dev.py stubs centralizes the stub testing logic and aligns with the updated dev.py dependencies (mypy==1.18.2).

Note: This change is related to the pipeline failure mentioned in dev.py. Ensure the issue with uncommitted files generated during stub testing is resolved before merging.

buildconfig/stubs/pygame/pixelarray.pyi (1)

36-36: Constructor stub switched to new. Aligns with similar pygame stubs for accurate mypy typing; confirm that PixelArray’s C implementation uses new so the stub matches runtime behavior.

@ankith26 ankith26 force-pushed the ankith26-3.14-typing branch from 1c20063 to a3ec9ac Compare October 8, 2025 08:17
Copy link
Member

@oddbookworm oddbookworm left a comment

Choose a reason for hiding this comment

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

Sure lol

Copy link
Contributor

@aatle aatle left a comment

Choose a reason for hiding this comment

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

LGTM

@aatle aatle added this to the 2.5.6 milestone Oct 9, 2025
@aatle aatle merged commit d601723 into main Oct 9, 2025
31 of 32 checks passed
@ankith26 ankith26 deleted the ankith26-3.14-typing branch October 9, 2025 03:50
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