Typing [Fixed #1489!]

[MarcelRobeer]

What does this change?

Added type annotation hints to all functions/classes in faker!

What was wrong

faker did not include type annotations (issue #1489).

How this fixes it

• Added automatic type annotations with monkeytype (https://monkeytype.readthedocs.io/en/latest/) for all files in which it did not throw errors.
• Manual linter fixes.
• Manual fixes of missing type annotations or unknown type annotations (e.g. type None). Did a lot of code analysis myself, but would also like to credit @nicarl for his earlier work on type annotations (e.g. in documentory.py and generator.py).
• mypy compliant (mypy -m faker). Also added mypy -m faker test to tox.ini!
• flake8 compliant (flake8 faker tests)
• isort compliant (python3 -m isort --check-only --diff .)

[nicarl]

Wow this is really great!
@fcurella I will have a look at this as I started adding type annotations some time ago.

[MarcelRobeer]

Thanks! Happy to help :)

I would like to point out some specific difficulties I faced, so you can better understand my decisions made:

1. f5a9985 & 64a2bfd: providers/__init__.py imports from generators and vice versa, resulting in a circular import error. I tried out a simple fix (i.e. only importing from .providers import BaseProvider in generator.py if typing.TYPE_CHECKING, but the ImportError then pops up when creating the coverage report on all OS-Python version combinations. As a current hotfix I replaced BaseProvider in generator.py with object. However, I would suggest for a future version to rethink generators having providers in them and providers linking to generators. Perhaps a factory pattern?
2. 28f7daa: typing.OrderedDict was newly introduced in Python 3.7.2, but will be deprecated again in Python 3.10 (https://www.python.org/dev/peps/pep-0585/). For now, in typing.py it attempts to import typing.OrderedDict and if this gets an ImportError (in Python 3.6) it falls back to the original MutableMapping. For Python 3.10 I would suggest compatibility checking the Python (minor) version and import collections.OrderedDict instead (which will support square bracket usage for typing []), e.g. in file typing.py:

import platform

major, minor, patchlevel = (int(version) for version in platform.python_version_tuple())

if major > 3 or major == 3 and minor >= 10:  # Python 3.10 and above
    from collections import OrderedDict
elif major == 3 and (minor < 7 or minor == 7 and patchlevel < 2):  # Python 3.7.1 and below
    from typing import MutableMapping as OrderedDict
else:
    from typing import OrderedDict

3. 1bb4e05: PyPy3 does not support mypy. It cannot even be pip-installed. Therefore, in tox.ini the install of the dependency was moved to within [testenv:mypy] and is skipped due to skip_missing_interpreters=true.

[MarcelRobeer]

@MarcelRobeer I more or less fixed all the problems (with some # type: ignore). Do you think you can cherry pick the two last commits from https://github.com/nicarl/faker/tree/fix_last_type_errors

• Did some minor changes
  • TypeVar was not used anymore in generator.py (made some of the # type: ignore redundant, which I removed)
  • Updated MANIFEST.in
  • Updated docs/coding_style.rst to include information on type hinting
  • Added mypy to .github/workflows/ci.yml

[nicarl]

Looks good :-)
Two open questions

• can we add the mypy step to the lint step of the CI?
• should we squash the commits before merging ?

[MarcelRobeer]

Looks good :-) Two open questions

• can we add the mypy step to the lint step of the CI?

I would suggest to keep this separate, since the lint step is only run on one Python version (3.9.7) and typing has had some major overhauls in 3.8 and 3.10. The current set-up runs mypy on 3.6, 3.8 and 3.10 to test all scenarios. Edit: Maybe we should add 'typing' to 'needs' in some places of the workflows/ci.yml file?

• should we squash the commits before merging ?

Yes, I agree we should do that. I think @fcurella should have an option 'squash and merge' to do that directly when applying it. Perhaps refer to docs/coding_style.rst in the description of that squashed commit?

[fcurella]

@MarcelRobeer I can do the squashing, no worries! :)

[fcurella]

I would suggest to keep this separate, since the lint step is only run on one Python version (3.9.7) and typing has had some major overhauls in 3.8 and 3.10. The current set-up runs mypy on 3.6, 3.8 and 3.10 to test all scenarios. Edit: Maybe we should add 'typing' to 'needs' in some places of the workflows/ci.yml file?

I agree in having it as a separate step. Ideally, I'd like to run it in parallel with the lint step, and before running the tests.

[MarcelRobeer]

I would suggest to keep this separate, since the lint step is only run on one Python version (3.9.7) and typing has had some major overhauls in 3.8 and 3.10. The current set-up runs mypy on 3.6, 3.8 and 3.10 to test all scenarios. Edit: Maybe we should add 'typing' to 'needs' in some places of the workflows/ci.yml file?

I agree in having it as a separate step. Ideally, I'd like to run it in parallel with the lint step, and before running the tests.

Implemented this. It now passes all tests. Note that I had to add the --no-warn-unused-ignores flag to mypy due to python/mypy#8823

[fcurella]

Thank you so much @MarcelRobeer , amazing work! ✨

@nicarl could you give this one last review?

[nicarl]

Just some last minor comments, once they are fixed it is good from my side 👍

[fcurella]

This is awesome! Thank you so much!

[obieler]

@nicarl @MarcelRobeer
Should the package typing-extensions be a dependency in setup.py?
typing-extensions>=3.6.4; python_version < "3.8"

[MarcelRobeer]

Not sure if typing-extensions is installed by default in earlier Python versions, or that it is installed on-the-go because of the --install-types flag for mypy in tox.ini? @nicarl do you know this by any chance?

When running tests this doesn't seem to form an issue; not sure if it pops up as an issue when running the code yourself?

[Edit 1]: looking at the typing (3.6) test it seems like it is installed together with other dependencies when setting up the python environment for tox:

Collecting typing-extensions>=3.6.4
     Downloading typing_extensions-3.10.0.2-py3-none-any.whl (26 kB)

[Edit 2]: Looks like typing-extensions is a dependency of mypy, which installs it by default: https://github.com/python/mypy/blob/f5fc579cf07f2078c9312044f6bcb132f891d746/setup.py

[obieler]

Apparently, Literal is included in typing only in Python 3.8 and above.
And typing_extensions is not included at least in Python 3.6, 3.7.

[nicarl]

@obieler thanks for digging that deep!

I just tried with Python 3.6.0 and 3.7.0 and typing_extensions is not contained.
Adding mypy to setup.py should solve the problem as it contains typing-extensions as dependency. At the same time one could pin mypy version to 0.910 as recommend in https://sethmlarson.dev/blog/2021-10-18/tests-arent-enough-case-study-after-adding-types-to-urllib3

Edit: opening a PR in a minute

[obieler]

I saw the fix in 9.5.1, thank you!