Get successful MyPy pass for azure-core

[rakshith91]

Resolves #5022

[adxsdk6]

Can one of the admins verify this patch?

[codecov-io]

Codecov Report

Merging #5266 into azure-core will increase coverage by <.01%.
The diff coverage is 87.64%.

@@              Coverage Diff               @@
##           azure-core    #5266      +/-   ##
==============================================
+ Coverage       49.63%   49.63%   +<.01%     
==============================================
  Files            6372     6372              
  Lines          179206   179214       +8     
==============================================
+ Hits            88944    88952       +8     
  Misses          90262    90262

Impacted Files	Coverage Δ
azure-core/azure/core/pipeline/base.py	85.71% <100%> (ø)	⬆️
...zure-core/azure/core/pipeline/policies/redirect.py	72.58% <100%> (ø)	⬆️
azure-core/azure/core/pipeline/transport/base.py	72.89% <100%> (ø)	⬆️
azure-core/azure/core/pipeline/__init__.py	66.66% <100%> (+1.96%)	⬆️
...re-core/azure/core/pipeline/policies/base_async.py	80.95% <100%> (ø)	⬆️
azure-core/azure/core/pipeline_client.py	36.84% <100%> (ø)	⬆️
azure-core/azure/core/pipeline/policies/base.py	96.29% <100%> (ø)	⬆️
...e-core/azure/core/pipeline/policies/credentials.py	100% <100%> (ø)	⬆️
azure-core/azure/core/pipeline_client_async.py	36.84% <100%> (ø)	⬆️
.../azure/core/pipeline/transport/requests_asyncio.py	54.79% <100%> (ø)	⬆️
... and 8 more

---

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 cab5b2a...3163de7. Read the comment docs.

[bryevdv]

@rakshith91 I am actually extremely new to MyPy, had been waiting to learn about it until later this year when other projects I run move to Python 3.5+. So you can help me understand a few things as I come up to speed:

• In general (not just this PR) I see a lot of complex types inline in code in lots of places, e.g List[Union[HTTPPolicy, SansIOHTTPPolicy]] Should/could we define type aliases more aggressively?

• also seeing more type: ignore on things like imports than I might naively expect to need. Are these temporary, and could be removed later as more modules are updated for mypy?

[rakshith91]

@rakshith91 I am actually extremely new to MyPy, had been waiting to learn about it until later this year when other projects I run move to Python 3.5+. So you can help me understand a few things as I come up to speed:

• In general (not just this PR) I see a lot of complex types inline in code in lots of places, e.g List[Union[HTTPPolicy, SansIOHTTPPolicy]] Should/could we define type aliases more aggressively?
• also seeing more type: ignore on things like imports than I might naively expect to need. Are these temporary, and could be removed later as more modules are updated for mypy?

@bryevdv

1. AFAIK, I don't think it's possible to define type aliases more aggressively. I can look more into it.

2. Those imports don't have stubs in typeshed. Writing our own stubs for 3rd party libraries would definitely be overkill. One alternative is we can ignore these in the command line arguments while setting it up in the CI and then remove these magic comments or wait till typeshed supports these libraries.

[bryevdv]

@rakshith91

By type more aggressively define type aliases, I mean only use this functionality more than we currently do: https://mypy.readthedocs.io/en/latest/kinds_of_types.html#type-aliases

So, in the specific case above, maybe: PolicesList = List[Union[HTTPPolicy, SansIOHTTPPolicy]] so that then we can use PoliciesList in place of the complicated expression in the actual code. As another example, this is duplicated 3-4 times:

type: Optional[Union[ServiceRequestError, ServiceResponseError]]

It seems like an alias for that would be both easier to parse (for a person) in the code, and also afford the ability to change things in one place instead of multiple places if the type is ever expanded or refined.

Thanks for the explanation re: typeshed, personally I think judiciously turning off some checks for certain things is useful (I wouldn't want to have to add some noisy comment just to import urllib) but that might be something for a wider discussion.

We could definitely make follow-on issues to discuss these things separately outside this PR.

[rakshith91]

Moved to #5466

Closing this as a duplicate.