Add mypy and typing_copilot checks to the CI pipeline. #1528
Conversation
|
cc @ColCarroll |
|
I think we can add CI checks in this same PR, but I don't really have a preference about this. Whatever works best for you. Can this be merged before #1504? |
|
Mypy version 0.800 changed how files for type-checking are selected, making more files eligible for type checking by default. This makes This is frustrating user experience, and I've opened obi1kenobi/typing_copilot#10 to attempt to improve the experience from I see a few possible options here, and would love to hear the thoughts of the maintainers:
My preference would be one of the first two options. I'd probably pick the first one myself, since adopting ratcheting and starting to enforce statically typed Python with |
|
Are these the only 7 errors? I like the first option. It looks like checking the examples folder is catching some false positives, |
Yes, there are the only 7 I'm seeing at the moment.
Sounds good to me! I'm happy to review and help you along the way, if you'd like. I keep finding myself going back to this link when I need to reference which abstract base class supports which methods: https://docs.python.org/3/library/collections.abc.html#collections-abstract-base-classes It shows that |
|
I have added #1581 to keep track of this. |
|
Regarding Sequence vs Iterable, in case it's of use to you, I find this little explanation from the mypy docs to be helpful:
So unless |
|
Mypy errors have been fixed, everything should now work even with latest mypy version. Should make adding the check to CI easier and not require modifying it after a while to unpin mypy. |
|
Unfortunately I ran into a regression in These are all unexpected (and cause an Unfortunately this is a serious regression that I don't think
|
Codecov Report
@@ Coverage Diff @@
## main #1528 +/- ##
=======================================
Coverage 90.86% 90.86%
=======================================
Files 108 108
Lines 11620 11620
=======================================
Hits 10558 10558
Misses 1062 1062 Continue to review full report at Codecov.
|
|
I have completed the checklist for this PR, I believe it is ready to merge at your convenience. Thanks for bearing with me, and apologies for the delay in getting it to the finish line. |
obi1kenobi
changed the title
| @@ -127,6 +127,14 @@ jobs: | |||
| python -m black --check arviz examples asv_benchmarks | |||
| displayName: 'black' | |||
|
|
|||
| - script: | | |||
| python -m mypy . | |||
There was a problem hiding this comment.
would it help with anything/is it possible to run mypy only on main arviz and example folders?
There was a problem hiding this comment.
Unfortunately, not at the moment. typing_copilot doesn't yet have the capability to run mypy with custom flags, so even if you made this command different, the typing_copilot step would still essentially run mypy . and still fail.
Hoping to fix this in a future version of typing_copilot since this is obviously going to be painful for many use cases.
|
It looks like I waited to long and now CI is not passing anymore. Here is the link: https://dev.azure.com/ArviZ/ArviZ/_build/results?buildId=4169&view=logs&j=af9a1d3a-ff8f-5483-6823-86518c773a9b&t=d1f9f1df-bd17-5f75-5a7b-8e272c593e8b&l=20. Any ideas @obi1kenobi? I can see all the modules mypy is complaining about have ignore imports in mypy.ini 😕 |
The missing type hint warnings aren't blocking. The failure is actually because of good news — type coverage has improved since the point in time when I ran To resolve, one can do one of two things:
I'm happy to open a PR to fix it, but this is a great opportunity to get some practice using |
|
Oh, cool! I'll give it a go now 😄 |
|
Updated at #1622 |
* Add a passing mypy.ini generated by typing_copilot. * Add a working mypy.ini file with mypy<0.800. * Add CI pipeline steps for mypy and typing_copilot checks.
Add the tightest-possible passing
mypy.inifile, autogenerated withtyping_copilotby runningtyping_copilot initfrom inside anarvizdevelopment environment witharvizinstalled within.Part of #1496. Next steps (in this PR or in separate, future PRs, per the maintainers' choice) would be:
mypycontinues to pass (i.e. the "no type-hint regressions" check)typing_copilot tighten --error-if-can-tightencontinues to pass (i.e. the "type hinting ratchet is as tight as possible" check)Technically,
typing_copilot tighten --error-if-can-tightenwill runmypywith the current settings first and therefore will duplicate some of the same work from the first check, but I'd recommend runningmypydirectly anyway just to be sure and avoid the risk oftyping_copilotbugs causing type-hinting quality to decrease unnoticed.