Skip to content

Remove obsolete "type: ignore" comment - #632

Draft
cuu508 wants to merge 1 commit into
aio-libs:masterfrom
cuu508:remove_obsolete_type_ignore
Draft

Remove obsolete "type: ignore" comment#632
cuu508 wants to merge 1 commit into
aio-libs:masterfrom
cuu508:remove_obsolete_type_ignore

Conversation

@cuu508

@cuu508 cuu508 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

python/typeshed#15742 is now fixed so this should not be needed any more

What do these changes do?

Removes a "type: ignore" comment which should no longer be needed.

Are there changes in behavior for the user?

No.

Related issue number

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • tox testenvs have been executed in the following environments:
    • Linux (Ubuntu 18.04, Ubuntu 20.04, Arch): {py36,py37,py38,py39}-{nocov,cov,diffcov}, qa, docs
    • Windows (7, 10): {py36,py37,py38,py39}-{nocov,cov,diffcov}
    • WSL 1.0 (Ubuntu 18.04): {py36,py37,py38,py39}-{nocov,cov,diffcov}, pypy3-{nocov,cov}, qa, docs
    • FreeBSD (12.2, 12.1, 11.4): {py36,pypy3}-{nocov,cov,diffcov}, qa
    • Cygwin: py36-{nocov,cov,diffcov}, qa, docs
  • Documentation reflects the changes
  • Add a news fragment into the NEWS.rst file
    • Add under the "aiosmtpd-next" section, creating one if necessary
      • You may create subsections to group the changes, if you like
    • Use full sentences with correct case and punctuation
    • Refer to relevant Issue if applicable

python/typeshed#15742 is now
fixed so this should not be needed any more
@cuu508
cuu508 marked this pull request as draft September 7, 2026 15:36
@codecov

codecov Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.81%. Comparing base (ea98162) to head (055aecf).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #632   +/-   ##
=======================================
  Coverage   97.81%   97.81%           
=======================================
  Files          23       23           
  Lines        5715     5715           
  Branches      352      352           
=======================================
  Hits         5590     5590           
  Misses         75       75           
  Partials       50       50           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cuu508

cuu508 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Ah, I was too impatient –

  • need to wait for a new mypy release which ships the typeshed fix
  • need to bump the mypy version in requirements-dev.txt

@greptile-apps

greptile-apps Bot commented Sep 7, 2026

Copy link
Copy Markdown

Confidence Score: 4/5

Not safe to merge until the Python 3.11 mypy error introduced by this change is resolved.

A before-and-after comparison using the pinned mypy version shows the parent source passes the focused check and the changed source fails specifically on cleanup_socket; the CI-equivalent comparison confirms this is a newly introduced error.

Files Needing Attention: aiosmtpd/controller.py needs a mypy-compatible annotation or compatibility wrapper at the Python 3.13 cleanup_socket=False call.

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex ran parent-focused and HEAD-focused mypy checks and produced a P1 finding proof; the parent run passed, while HEAD failed on cleanup_socket.
  • T-Rex created reproduction scripts for the CI mypy checks on both the parent and HEAD branches and used them to reproduce the behavior observed in the proofs.
  • T-Rex executed the focused Unix-socket tests and confirmed HEAD Unix-socket controller tests pass.
  • T-Rex performed general-contract validation, showing static-before and static-after results, a CI comparison, and runtime verification of the Unix-socket tests.
  • T-Rex posted a second finding-comment proof for another P1 finding.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (1)

  1. General comment

    P1 Removing the mypy suppression breaks the configured lint check

    • Bug
      • The changed create_unix_server(..., cleanup_socket=False) call produces mypy 1.10.1 error [call-arg] at aiosmtpd/controller.py:456:20. The parent implementation passes the same focused check; the configured GitHub Actions lint job uses Python 3.11 and invokes this pinned mypy version without a target-version override.
    • Cause
      • The Python 3.11/typeshed signature for asyncio.AbstractEventLoop.create_unix_server does not declare the Python 3.13-only cleanup_socket keyword. Mypy type-checks the guarded branch and does not narrow its library signature based on sys.version_info for this call.
    • Fix
      • Restore the targeted # type: ignore[call-arg] on cleanup_socket=False, or use another mypy-compatible compatibility wrapper/cast while retaining the Python 3.13 runtime guard.

    T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "Remove obsolete "type: ignore" comment" | Re-trigger Greptile

Comment thread aiosmtpd/controller.py
# Silence mypy warning as cleanup_socket is not yet annotated in
# typeshed: https://github.com/python/typeshed/issues/15742
cleanup_socket=False, # type: ignore[call-arg]
cleanup_socket=False,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Mypy suppression breaks lint

The project’s Python 3.11 lint job uses pinned mypy==1.10.1, whose available create_unix_server signature does not include the Python 3.13-only cleanup_socket parameter. Removing this targeted suppression adds an Unexpected keyword argument "cleanup_socket" [call-arg] failure even though the branch is runtime-guarded. Restore the # type: ignore[call-arg] or use an equivalent mypy-compatible compatibility wrapper.

Artifacts

Parent focused mypy comparison script

  • Authored shell script extracts the parent controller and runs the pinned focused mypy check, ending with the check's exit status.

HEAD focused mypy script

  • Authored shell script runs the pinned focused mypy check against the changed controller, ending with the check's exit status.

Parent focused mypy check passes

  • Captured parent-source mypy output reports no issues and exit code 0, establishing the before condition.

HEAD focused mypy check fails on cleanup_socket

  • Captured changed-source mypy output reports the unexpected cleanup_socket keyword at controller.py:456 and exit code 1, proving the regression.

Parent CI mypy reproduction script

  • Authored shell script exports the parent tree and runs the CI-equivalent bare mypy command, ending with the command's exit status.

HEAD CI mypy reproduction script

  • Authored shell script runs the CI-equivalent bare mypy command in the changed checkout, ending with the command's exit status.

Parent CI mypy baseline output

  • Captured parent full-mypy output contains five pre-existing missing-stub errors and exit code 1, establishing the CI baseline.

HEAD CI mypy output adds cleanup_socket failure

  • Captured changed full-mypy output contains the five baseline errors plus the new controller cleanup_socket call-argument error, proving the additional failure.

Focused Unix-socket test script

  • Authored shell script runs the repository's focused Unix-socket controller tests and records their exit status.

HEAD Unix-socket controller tests pass

  • Captured focused pytest output shows all three Unix-socket controller tests passing on Python 3.11, confirming unchanged reachable runtime behavior.

View artifacts

T-Rex Ran code and verified through T-Rex

@webknjaz

webknjaz commented Sep 8, 2026

Copy link
Copy Markdown
Member

Bear in mind that we'll hopefully run MyPy against a range of Python versions so depending on how far back the upstream fix gets backported, we'll need to keep having the ignore in place but track the respective date for when it could get removed for good (maybe via https://endoflife.date/python + an issue).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants