Mypy type checking

[qgallouedec]

Description

Motivation and Context

• I have raised an issue to propose this change (required for new features and bug fixes)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation (update in the documentation)

Checklist:

• I've read the CONTRIBUTION guide (required)
• I have updated the changelog accordingly (required).
• My change requires a change to the documentation.
• I have updated the tests accordingly (required for a bug fix or a new feature).
• I have updated the documentation accordingly.
• I have reformatted the code using make format (required)
• I have checked the codestyle using make check-codestyle and make lint (required)
• I have ensured make pytest and make type both pass. (required)
• I have checked that the documentation builds using make doc (required)

Note: You can run most of the checks using make commit-checks.

Note: we are using a maximum length of 127 characters per line

[araffin]

I would also add a mypy entry (would make debugging easier as pytype is quite slow as soon as you change one file)

[araffin]

I would also activate the color output as we do for pytest, see python/mypy#7771

[qgallouedec]

I would also add a mypy entry (would make debugging easier as pytype is quite slow as soon as you change one file)

Should I replace type by pytype then?

[araffin]

type is still fine as we want to run both in the CI or when developping.

[qgallouedec]

I would also activate the color output as we do for pytest, see python/mypy#7771

I'm not sure to use MYPY_FORCE_COLOR=1 properly, it does not seem to work as expected.

[araffin]

yep, I saw that :/ will see if I can find an example online

[qgallouedec]

What about this @araffin?

[araffin]

I double check and you seem to be doing things right...

[qgallouedec]

I need help here. I can't understand how to ignore files in setup.cfg. The current config seems to work locally, but sometimes it doesn't. I've failed so far to get a reproductible procedure.

[Rocamonde]

What we do in imitation is we define a variable called EXCLUDE_MYPY
https://github.com/HumanCompatibleAI/imitation/blob/d6f8ca2748981deecd5b660b3c665ae98c92b56b/ci/code_checks.sh#L5
then we pass this as an arg when we run mypy
https://github.com/HumanCompatibleAI/imitation/blob/d6f8ca2748981deecd5b660b3c665ae98c92b56b/ci/code_checks.sh#L37
crucially you need to pass --follow-imports=silent otherwise it will follow imports and still raise errors for the files that you're ignoring. --show-error-codes is also useful.

We also skipped using setup.cfg, for a while we used mypy.ini which seemed to work consistently but now we just ignore the files manually on CI calls. This is so that when you're developing and run mypy you still get error messages, but not when the CI is run.

[qgallouedec]

crucially you need to pass --follow-imports=silent otherwise it will follow imports and still raise errors for the files that you're ignoring

It works! Thanks!

[qgallouedec]

Except for colored output, (#1143 (comment)), it's ready for review.

[araffin]

From my part, I don't have much to say, LGTM =)

but I will let @Rocamonde comment on it and he is the one experienced with such transition.

[araffin]

I will merge this as it prevents from fixing type annotations.
@Rocamonde if you have time in the coming days, feel free to do a "post-review".

For the color issue, it's minor so let's fix it later if we find out what we are doing wrong.