Enable mypy in CI

[Dreamsorcerer]

Initial mypy config and type fixes to get it enabled in CI.

Starting on this while I'm waiting on some other things.

[lgtm-com]

This pull request introduces 1 alert when merging 9656b42 into 04fefd8 - view on LGTM.com

new alerts:

• 1 for Unused import

[lgtm-com]

This pull request fixes 1 alert when merging fe7547d into 04fefd8 - view on LGTM.com

fixed alerts:

• 1 for Unused import

[seandstewart]

@Dreamsorcerer -

Thanks for working in this for us! I've added @bmerry as a reviewer, as he's done a lot of work to get our static typing up to snuff.

[lgtm-com]

This pull request fixes 1 alert when merging f8d83df into 04fefd8 - view on LGTM.com

fixed alerts:

• 1 for Unused import

[Dreamsorcerer]

Commented out things are to be enabled in future PRs.

[codecov]

Codecov Report

Merging #1101 (6180fa8) into master (04fefd8) will decrease coverage by 0.08%.
The diff coverage is 85.50%.

@@            Coverage Diff             @@
##           master    #1101      +/-   ##
==========================================
- Coverage   89.22%   89.14%   -0.09%     
==========================================
  Files          21       21              
  Lines        6823     6871      +48     
  Branches      653      658       +5     
==========================================
+ Hits         6088     6125      +37     
- Misses        567      580      +13     
+ Partials      168      166       -2     

Flag	Coverage Δ
unit	89.09% <85.00%> (-0.10%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
aioredis/connection.py	65.91% <68.96%> (-0.35%)	⬇️
aioredis/utils.py	58.06% <80.00%> (+3.89%)	⬆️
aioredis/client.py	82.54% <92.80%> (+0.23%)	⬆️
aioredis/compat.py	100.00% <100.00%> (ø)
aioredis/sentinel.py	80.40% <100.00%> (ø)
tests/test_connection.py	69.23% <0.00%> (+7.32%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 04fefd8...6180fa8. Read the comment docs.

[lgtm-com]

This pull request fixes 1 alert when merging ed06034 into 04fefd8 - view on LGTM.com

fixed alerts:

• 1 for Unused import

[lgtm-com]

This pull request fixes 1 alert when merging bea076b into 04fefd8 - view on LGTM.com

fixed alerts:

• 1 for Unused import

[lgtm-com]

This pull request fixes 1 alert when merging 8617538 into 04fefd8 - view on LGTM.com

fixed alerts:

• 1 for Unused import

[Dreamsorcerer]

client.py is almost done, will probably push it tomorrow.

Could one of you do a quick review of my PR to upgrade aioredis in aiohttp-session?
aio-libs/aiohttp-session#611

[bmerry]

I'm afraid I'm not going to have time to review this, sorry.

[lgtm-com]

This pull request fixes 1 alert when merging e8e21a4 into 9d71774 - view on LGTM.com

fixed alerts:

• 1 for Unused import

[lgtm-com]

This pull request introduces 1 alert and fixes 1 when merging e7d456d into 9d71774 - view on LGTM.com

new alerts:

• 1 for Unused import

fixed alerts:

• 1 for Unused import

[lgtm-com]

This pull request fixes 1 alert when merging 250e6ed into 9d71774 - view on LGTM.com

fixed alerts:

• 1 for Unused import

[lgtm-com]

This pull request fixes 1 alert when merging ae2dfa6 into 9d71774 - view on LGTM.com

fixed alerts:

• 1 for Unused import

[lgtm-com]

This pull request fixes 1 alert when merging f613529 into 9d71774 - view on LGTM.com

fixed alerts:

• 1 for Unused import

[lgtm-com]

This pull request fixes 1 alert when merging f5a8e80 into 9d71774 - view on LGTM.com

fixed alerts:

• 1 for Unused import

[Dreamsorcerer]

I wouldn't worry too much about reviewing all the type annotations. Any mistakes should become apparent and be fixed as we expand the typing coverage. If someone can review the minor runtime changes, that's probably all that's really needed before merging this.

I'll wait until you're ready to proceed before making any more progress.

[Andrew-Chen-Wang]

Looks fine to me. Most of this code is just typing, but there are some stuff that isn't; for example, the "cast"-ing is something I'm not really happy with. Or an assertion statement that should instead be an if condition.

Besides that, I think this PR doesn't affect the code base too much.

Though, when it comes to maintenance and PR adding, I do think the CI could be non-blocking like codecov. It'll obviously break people's mypy every so often, but I guess it's a balancing act.

[Andrew-Chen-Wang]

Why is this enum.Enum needed? And can you prepend an underscore to Sentinel?

[Dreamsorcerer]

Sentinels are a little tricky to type. It seems that an enum is the best approach to use:
https://stackoverflow.com/a/60605919

[Dreamsorcerer]

for example, the "cast"-ing is something I'm not really happy with.

Note that casting is just typing. At runtime it just returns the argument unchanged.
Although I do try to avoid casting unless the only other option is to ignore the errors.

Though, when it comes to maintenance and PR adding, I do think the CI could be non-blocking like codecov. It'll obviously break people's mypy every so often, but I guess it's a balancing act.

I think the problem is that the easiest time to fix a typing error is when you make the change. i.e. The author of a PR has the best knowledge of that code during the period of working on that PR.
For example, me trying to fix the typing errors now is a lot trickier as I have to keep making assumptions and guesses at the intended behaviour. I'm sure there are some typing mistakes in there which will be revealed in later PRs when the typing is expanded.

[Andrew-Chen-Wang]

Note that casting is just typing. At runtime it just returns the argument unchanged.
Although I do try to avoid casting unless the only other option is to ignore the errors.

I was just thinking it as an extra, "unnecessary" operation in terms of actual execution of commands.

I think the problem is that the easiest time to fix a typing error is when you make the change. i.e. The author of a PR has the best knowledge of that code during the period of working on that PR.

That's true. @abrookins Can you provide a review of in general 1. should we still include the mypy CI (I approve) and 2. any other concerns? I wouldn't bother reviewing the entire thing since we can just change annotations over time if there is something incorrect.

[lgtm-com]

This pull request fixes 1 alert when merging 6180fa8 into 4fbee2e - view on LGTM.com

fixed alerts:

• 1 for Unused import

[Andrew-Chen-Wang]

Thanks a lot @Dreamsorcerer !

[Dreamsorcerer]

Oh man, I'm trying to add the examples to the type checking and aioredis doesn't lend itself to static type checking very well.

The examples use Pipeline which inherits from Redis. However, Redis.get() etc. return an Awaitable, but when using the same method on Pipeline, it may return a Pipeline. This violates the Liskoff Substitution Principle and makes it very hard, maybe impossible, to type it correctly. The return type on Pipeline.execute_command() is also Union[Pipeline, Awaitable[Pipeline]], which is basically useless to the user as they will always get an error when using it.