Force type hints

[finswimmer]

This PR add missing type hints to both package code and test code. Furthermore flake-annotations is added as a flake8 plugin to enforce type hints in the future.

[neersighted]

This whole file is very cargo-culted. Can we clean it up while we're in here?

[neersighted]

Wow, this is a complicated type. Is there any way to clean this up with NewType, a refactor with an object, or similar?

[finswimmer]

I was happy, that PyCharm was doing this for me 😆 IMO refactoring should be done in another PR, because the purpose of this PR is to enforce type hints in the future.

[neersighted]

That is a brutal type signature :)

[finswimmer]

I've tried to understand the code and IMO the signature can be written more easily. See code change.

[danieleades]

mypy itself can be configured to return errors on missing annotations, i believe. is the 'flake8' plugin somehow preferable?

• if TYPE_CHECKING pattern doesn't appear to be required throughout
• inconsistent use of dict/Dict

[danieleades]

you shouldn't need the if TYPE_CHECKING pattern here- that's really only for preventing circular imports. You can just import these directly, and use them below without quoting the types

[finswimmer]

That's a convention I'm used to. If you need a module just for type hints, import it only in the if TYPE_CHECKING block.

It's easier to teach to other people ("Why are you sometime put import in that block and sometimes not?") and the overall chance that you have a circular import if you need this import just for type hint is quite big.

TL;DR: I prefer to leave it as it is.

[danieleades]

fair enough. there's some interesting background on it here - larryhastings/co_annotations#1

in general, it seems that if TYPE_CHECKING is used iff

• you would otherwise have a circular import
• the import is 'expensive'

the Benevolent Dictator for Life seems to regard it is a painfully necessary hack.

that said, it's also clear from the thread that it's far from settled. And using it everywhere instead of the small number of cases where you need it (do you definitely need it anywhere in this codebase?) is at least consistent.

[danieleades]

you can use Config unquoted if you remove the if TYPE_CHECKING pattern above

[danieleades]

this syntax is python 3.9+. less than that, the correct return type would be Dict, rather than dict.

I guess we can get away with it, since mypy is being run with a recent interpreter, and the type is ignored during other testing, but whichever way we go, it should be consistent throughout the codebase

[danieleades]

while we're at it, these dict annotations are equivalent to Dict[Any, Any]- we should be able to do better than that

[finswimmer]

As you find out dict is valid as well, but don't let us define how keys and values look like in python<3.9. I agree it's in inconsistent use if we use Dict in other places, so I changed it.

while we're at it, these dict annotations are equivalent to Dict[Any, Any]- we should be able to do better than that

The only way to do it better here is to use Dict[str, Any] (which doesn't add much benefit) or use TypedDict. But this is only available for python >=3.8 and would require to add typing-extensions as dependency for python <3.8. Not sure if we want to introduce another dependency.

[danieleades]

is there a circular import if you do not guard the import of MockerFixture?

[danieleades]

if you remove the if TYPE_CHECKING guard, this can be written as-

Suggested change

	def parse(
	cls, value: str, version_class: Optional[Type["PEP440Version"]] = None
	) -> "PEP440Version":
	def parse(
	cls, value: str, version_class: Optional[PEP440Version] = None
	) -> PEP440Version:

[danieleades]

might be a good idea to merge #199 before this is merged, in order to catch any type errors that could be interested by this PR

[neersighted]

re: if TYPE_CHECKING, check out https://github.com/asottile/flake8-typing-imports

Looks like a useful set of lints we could apply here.