Skip to content

Conversation

@jfhbrook
Copy link
Owner

@jfhbrook jfhbrook commented Mar 17, 2025

#176 shows that mypy's behavior deviates significantly from pyright, to the point that we should QA against it.

This PR also includes a bunch of typing fixes - mostly for the benefit of mypy, though a few fixes for pyright as well. In the latter case, pyright expects the Self type now. Note that this means I'd have to drop support for 3.8, 3.9 and 3.10 - unless, of course, I define Self = Any.

QA passes, but I need to decide whether or not I want to cut a major release.

@jfhbrook jfhbrook changed the title Add mypy to CI Address mypy type checking errors Mar 17, 2025
Copy link
Owner Author

@jfhbrook jfhbrook left a comment

Choose a reason for hiding this comment

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

Just some notes to self.

pyee/twisted.py Outdated
self.emit("failure", Failure())
else:
if iscoroutine and iscoroutine(result):
if iscoroutine and iscoroutine(result): # type: ignore
Copy link
Owner Author

Choose a reason for hiding this comment

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

mypy thinks iscoroutine will always return True. I don't know why it thinks this.

But also, if I drop the conditional import, I shouldn't need to check for iscoroutine.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I think it was actually that iscoroutine would always be truthy. Getting rid of the conditional import may fix this.

@jfhbrook
Copy link
Owner Author

jfhbrook commented Mar 17, 2025

I decided to use Self = Any in the typing, but to release as a major due to other changes:

  • Overloads for ee.on and various un-annotated None return types
  • Removed conditional import of iscoroutine, initially introduced for Python 3.3 support
  • Removed twisted.python.Failure type stub, initially introduced due to typing bugs in old versions of Twisted
  • Addition of mypy to dev dependencies - these are treated as true dependencies for a "dev" feature in pypi

Once this is merged, I'll make new PRs which add a true Self type and hide some private members of the pyee.cls module, and I'll cut an issue for removing the dev dependency group.

@jfhbrook jfhbrook merged commit d0aa115 into main Mar 17, 2025
8 checks passed
@jfhbrook jfhbrook deleted the mypy-ci branch March 17, 2025 18:51
@jfhbrook jfhbrook mentioned this pull request Mar 17, 2025
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.

3 participants