Strict requirement for type hinting

[jacobcook1995]

Description

Based on the discussion in #60 it seems like a good idea to make type annotations mandatory. If this becomes a problem down the we can always switch to a less strict option, but if we are going to use a more strict option, switching to it down the line would be a nightmare (there already were a lot of extra mypy errors to fix with our currently pretty minimal code base).

I could find type stubs for either shapely or dpath, so turned off type checking when I imported them. I may not have been looking in the right places for them though. So, if anyone knows where they can be found, let me know.

Type of change

• New feature (non-breaking change which adds functionality)
• Optimization (back-end change that speeds up the code)
• Bug fix (non-breaking change which fixes an issue)

Key checklist

• Make sure you've run the pre-commit checks: $ pre-commit run -a
• All tests pass: $ poetry run pytest

Further checks

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works

[davidorme]

It looks like shapely is planning on implementing but isn't there yet (shapely/shapely#721) but I can find nothing on dpath. Both of those could be suppressed at the project level in the cfg file, but we can look at that if we end up using these a lot in different modules, rather than just grid and config.

[davidorme]

It looks like we should be able to use conftest.py to configure mypy to be less strict in the test suite than the main code? Do we really need to enforce typing on test functions? I guess it doesn't hurt, but it is finnicky.

[jacobcook1995]

Yeah I wasn't sure whether or not requiring type hints on testing code is desirable, but that can be changed if we decide that it isn't

[davidorme]

I have had a look at this, because my next PR (#84) is affected. We can include the code below in tests/conftest.py to make mypy more forgiving on the tests.

def pytest_configure(config):
    plugin = config.pluginmanager.getplugin("mypy")
    plugin.mypy_argv.append("--allow-untyped-calls")
    plugin.mypy_argv.append("--allow-untyped-defs")
    plugin.mypy_argv.append("--allow-incomplete-defs")

I think this is probably worth doing - I guess it is a similar argument to whether or not you use docstrings. I think docstrings are worthwhile because it is the 'right' place to comment on tests when needed, but I'm not sure we need to enforce typing on tests. @dalonsoa and @alexdewar ?

[dalonsoa]

I think that being more relaxed with typing in the tests make sense, so if there is a way of removing the most annoying aspects of mypy there, as it seems there is, I would use it.

[davidorme]

OK. With that feedback, I'd say we roll back the typing added in tests and then add the local pytest-mypy plugin configuration to conftest.py.

[alexdewar]

I agree. The only caveat I would add though is that if it ends up being a faff to exclude the tests folder from the mypy linting, you could just leave the type hints there. In practice this will just mean adding a bunch of -> Nones, which isn't that big a deal.

[jacobcook1995]

Actually appears to be fairly straightforward to add module/folder level exclusions for mypy in setup.cfg

[davidorme]

That config solution looks good - a bit cleaner! I've applied this branch to #84 and it works, so I'm happy to approve this.