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

Type-checking and mark package as typed #27

Merged
merged 1 commit into from
Nov 2, 2022
Merged

Conversation

Avasam
Copy link
Collaborator

@Avasam Avasam commented Oct 30, 2022

Mark pywinctl as typed using the py.typed marker. See https://peps.python.org/pep-0561/
This allows static type checkers like pyright, mypy, pytype, pycharm, etc. to use inline types instead of looking for a non-existent type-stub.

I've added Github actions to validate typing using pyright and mypy.
Checking all python versions may be overkill. Testing only the oldest supported may be enough.
CI validation: Avasam#1

And finally I've fixed any typing issue from basic type-checking.

@Kalmat
Copy link
Owner

Kalmat commented Oct 31, 2022

Hi!
First off, thank you so much for your help and interest!!
Excuse my ignorance... what is this all about? I mean, what are mypy, pyright and type-checking; and what are they useful for (I read the PEP-0561 article, but wasn't able to fully understand, sorry)? Should I merge it now or should I wait until all work is done (it says "work in progress")?
Thank you again!

@Avasam
Copy link
Collaborator Author

Avasam commented Oct 31, 2022

It is work in progress, so no need to merge now. I would also highly suggest reviewing the changes to ensure they all make sense to you, why a change had to be made, and to ensure I respected the intent of certain logic.

For example, when a None window title on MacOS is about to be used it a way that would raise an error. Should we return early? Is it fine to consider it equivalent to an empty string? (it depends on the use-case and intent, since a None window title has a special meaning).


What is / Why type-checking?

Static type checking is about evaluating which type your variables should have at any given point. As well as classes and method signatures etc. It is not a linter (like Flake8, pylint, pycodestyle, etc.) or a formatter (like autopep8, isort, Black, pycln, etc.)
It allows you to, during development, ensure that you're using sane types, keep track of what your variables are, and not accidentally misuse them. Resulting in more resilient code, and a more truthful and complete API for libraries:

# Very simple examples
a = 0
a.lower()  # Errors at runtime. Type checkers will catch this.

b: str
b = 0 # type checkers won't accept this

c: str | int
c = 0  # This is fine
c = ""  # This is fine

What are the tools mentioned?

https://google.github.io/pytype/ is a very simple and minimal checker by google that only runs on linux.
http://mypy-lang.org/ is the "official" python type-checking tool. But is often falling behind, has edge-case bugs, or doesn't have full support.
https://github.com/microsoft/pyright is Eric Traut (Microsoft)'s python type checker written in TypeScript. Really good and powerful.
The PyCharm (by Jetbrain) editor comes with its own type-checking tool


What are stubs? (PEP 561). And typeshed?

Type stubs are .pyi files that contain no functional code, only type definitions.
They're usually third-party packages to type libraries that are missing typing definitions. They can also be useful to describe c-extensions. Typeshed is the official central python repository of 3rd-party type stubs that everyone can contribute to. Typeshed actively supports all 4 tools mentioned above.

types-* packages are third-party stubs uploaded by typeshed

Libraries have the option to mark their packages as "typed" using the py.typed marker file. When that's done, type-checkers stop looking for 3rd-party stubs, and instead trust the package to provide their own types. Removing the following error:
image
image
It also automatically adds the following tag on PyPI:
image


Why these changes?

  1. Adding the py.typed marker file is for the reason mentioned above. Most of this library already provides inline-typing. It is much better to leverage that, than create a third-party stub.
  2. Added CI actions to run mypy and pyright. The 2 most popular (and best) type-checkers. To help validate that the types and signatures this library provide are sane. If this library is gonna mark itself as using inline-type, let's ensure the types are actually correct. It also helps ensure support for specific Python versions isn't broken.
  3. Even in basic type-checking mode. Said tools found a couple issues. Like accessing potentially None values, unused imports, obsolete typing code, missing | None (used to be Optional[]) on parameters, comparisons with an always None value, methods exposing the wrong return type, etc.
    So I'm also fixing everything found said tools in their non-strict mode so that the CI can pass.
  4. Updated dev requirements: mypy doesn't come bundled with typeshed (pyright does). So we need to specify the extra type-stubs explicitly.

How to type-check locally?

On VSCode:
You can install the Python and Pylance extensions. Make sure that mypy is turned on in the settings.

On PyCharm:
It has it's own integrated type-checkers. There's mypy plugin(s?), but I don't use the editor so I can't vouch for any.

As a command line:
Mypy:

  1. Install mypy: pip install mypy
  2. mypy .

Pyright:

  1. Install NodeJS
  2. npx pyright

@Avasam Avasam force-pushed the py.typed branch 3 times, most recently from f4fda84 to 77e3903 Compare October 31, 2022 23:01
@Avasam
Copy link
Collaborator Author

Avasam commented Oct 31, 2022

I still need to add NamedTuples to types-pywin32 for the CI to pass (Avasam#1). Other than that, ready for review..

@Avasam Avasam marked this pull request as ready for review October 31, 2022 23:05
Added `py.typed` marker
Added CI action to validate against mypy and pyright
Fix type issues for basic typing
@Kalmat
Copy link
Owner

Kalmat commented Nov 2, 2022

Woooow!!! Sorry for making you write that awesome explanation. I'm merging the PR right now! I will have to test everything before uploading it to PyPi.

Thanks a LOT!!!!

@Kalmat Kalmat merged commit 8c1f8ea into Kalmat:master Nov 2, 2022
@Avasam Avasam deleted the py.typed branch November 2, 2022 14:15
@Avasam Avasam mentioned this pull request Nov 3, 2022
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.

None yet

2 participants