Add basic type hints

[kasium]

❓ What kind of change does this PR introduce?

• 🐞 bug fix
• 🐣 feature
• 📋 docs update
• 📋 tests/coverage improvement
• 📋 refactoring
• 💥 other

📋 What is the related issue number (starting with #)

#384

❓ What is the current behavior? (You can also link to an open issue here)

#384

❓ What is the new behavior (if this is a feature change)?

no new behavior

📋 Other information:

📋 Checklist:

• I think the code is well written
• I wrote good commit messages
• I have squashed related commits together after the changes have been approved
• Unit tests for the changes exist
• Integration tests for the changes exist (if applicable)
• I used the same coding conventions as the rest of the project
• The new code doesn't generate linter offenses
• Documentation reflects the changes
• The PR relates to only one subject with a clear title
  and description in grammatically correct, complete sentences

---

This change is 

[kasium]

So I gave some simple type hints a try. However I cannot active them in CI yet, since mypy will check the ,py files as well as the pyi ones.
Do you think it make sense to only let mypy run on pyi files?

[kasium]

@webknjaz can you please have a look?

[webknjaz]

This PR needs rebasing and I've also added a few notes on things that need to be fixed. Could you look into it?

[codecov]

Codecov Report

Merging #385 (786aa4b) into master (ebbe86a) will decrease coverage by 1.07%.
The diff coverage is n/a.

❗ Current head 786aa4b differs from pull request most recent head bc6d139. Consider uploading reports for the commit bc6d139 to get more accurate results

@@            Coverage Diff             @@
##           master     #385      +/-   ##
==========================================
- Coverage   80.50%   79.43%   -1.08%     
==========================================
  Files          28       28              
  Lines        4390     4390              
==========================================
- Hits         3534     3487      -47     
- Misses        856      903      +47     

[kasium]

So, running mypy will now fail:

# pre-commit run mypy -a
mypy.....................................................................Failed
- hook id: mypy
- exit code: 2

cheroot/__init__.pyi: error: Duplicate module named "cheroot" (also at
"cheroot/__init__.py")
cheroot/__init__.pyi: note: Are you missing an __init__.py? Alternatively, consider using --exclude to avoid checking one of them.
Found 1 error in 1 file (errors prevented further checking)

Either we exclude the py files or we use stubtest to check them against the real files (https://stackoverflow.com/questions/51716200/how-do-you-check-if-a-typeshed-stub-pyi-file-matches-the-implementation)

[webknjaz]

So, running mypy will now fail:

# pre-commit run mypy -a
mypy.....................................................................Failed
- hook id: mypy
- exit code: 2

cheroot/__init__.pyi: error: Duplicate module named "cheroot" (also at
"cheroot/__init__.py")
cheroot/__init__.pyi: note: Are you missing an __init__.py? Alternatively, consider using --exclude to avoid checking one of them.
Found 1 error in 1 file (errors prevented further checking)

Either we exclude the py files or we use stubtest to check them against the real files (stackoverflow.com/questions/51716200/how-do-you-check-if-a-typeshed-stub-pyi-file-matches-the-implementation)

It's a side-effect of passing files as individual args to mypy. This way it works with separate contexts not realizing that the whole package should be treated as one whole.
The solution is to tell pre-commit not to pass those files but lint the whole thing. Here's a few more examples — https://github.com/sanitizers/octomachinery/blob/be18b54/.pre-commit-config.yaml#L60-L79 and https://github.com/abhinavsingh/proxy.py/blob/1eeed91/.pre-commit-config.yaml#L159-L183. Please copy-paste and edit the latter — it already has the style I want (no unnecessary quotes, right+consistent indentation, multiline sequences). But the key part is that pass_filenames: false combined with -p <package name>.
Once you do this, you can verify the check locally with something like tox -e pre-commit -- mypy --all-files.

When this is ready and works well, you'll need to duplicate the mypy entry (the hook part inside the repo) and invoke it with Python 2 while the first one would be invoked with Python 3.

P.S. You may keep --install-types but you'll still need to list them in additional_dependencies for pre-commit.ci integration to work because that env runs the checks in a disconnected from the network environment.

[kasium]

It's a side-effect of passing files as individual args to mypy. This way it works with separate contexts not realizing that the whole package should be treated as one whole. The solution is to tell pre-commit not to pass those files but lint the whole thing. Here's a few more examples — https://github.com/sanitizers/octomachinery/blob/be18b54/.pre-commit-config.yaml#L60-L79 and https://github.com/abhinavsingh/proxy.py/blob/1eeed91/.pre-commit-config.yaml#L159-L183. Please copy-paste and edit the latter — it already has the style I want (no unnecessary quotes, right+consistent indentation, multiline sequences). But the key part is that pass_filenames: false combined with -p <package name>. Once you do this, you can verify the check locally with something like tox -e pre-commit -- mypy --all-files.

done

When this is ready and works well, you'll need to duplicate the mypy entry (the hook part inside the repo) and invoke it with Python 2 while the first one would be invoked with Python 3.

Sorry, I don't get this point

P.S. You may keep --install-types but you'll still need to list them in additional_dependencies for pre-commit.ci integration to work because that env runs the checks in a disconnected from the network environment.

I added what's possible but for some dependencies there are no types...

[kasium]

@webknjaz okay, config is now ready. However, do you really want to check the .py files if there is no pyi file?
What do you think about just to add all missing types (with first a lot of Any's) by using mypy stubgen?

[webknjaz]

@webknjaz okay, config is now ready. However, do you really want to check the .py files if there is no pyi file?

I think so. It should ease the further mypy adoption, right?

What do you think about just to add all missing types (with first a lot of Any's) by using mypy stubgen?

You could try that.

[webknjaz]

Re: type[no-redef] — this is a known bug in mypy with try/except import fallbacks style. It can be sorted by replacing that with if/else. There is a bug on their tracker, I don't remember the exact number.

[webknjaz]

@kasium thanks again for working on this!
It seems like there are a few failures that are visible at https://results.pre-commit.ci/run/github/16620627/1638950716.FddrDhgSRnSAcxDS8WIZng. I mentioned earlier (#385 (comment)) that the resolution of one of them is changing try/except to if/else. I haven't checked closely but the others may need type-ignores added (unless there's a way to come up with a proper solution, that is).

Also, I wanted to ask if you were planning to apply interactive rebase to your PR branch when you're done to clean it up a bit? If not, I'll likely do this myself before merging. Please tell me what your preference is.

[kasium]

@webknjaz I'll take a look into the issue. If it's fine´, I'll leave the rebase up tp you (however, a squash also helps).
One other thing: Currently mypy just checks the stubs, but not the stubs against the code base (stubtest). I guess we should include that, right?

[webknjaz]

Yes, it would be great to include that type of testing as well. I just want to be conscious about the PR size and the scope creep. It's probably easier to have a series of small PRs rather than a big one — it's easier to digest from the reviewer's perspective and easier to merge complete changes faster rather than having some finished parts of the change wait for one unfinished one. It'd also help for GHA to run automatically on your PRs (GitHub requires manual approval from me for the first-time contributor PRs but post-merge, the new PRs won't need that).

[kasium]

@webknjaz I hope, I now solved the missing pieces. I guess it makes sense to have a general issue with some tasks

• Add typing to _compat
• Add typing to tests (?)
• Activate "warn_usused_ignore" (needed because of differences in py2 and py3)
• Add stubtest
• Add py.typed file and add it to the distribution
• Improve the stubs (add missing types and remove Any's)

This all can be done in separate PRs

[webknjaz]

Great job! I'll merge this soon.

As for the tracking issue, that sounds reasonable, feel free to file it.

[webknjaz]

Looks like GHA has broken one of their images so I'll have to merge without green CI.