mypy will never honor dependencies

[jaraco]

The current mypy config currently ignores dependencies by (a) not installing them and then (b) ignoring missing imports for those dependencies.

The non-installation of dependencies means that even for projects that have since added and declared typing support (both keyring and importlib_metadata fall into this boat), they're not tested.

I did find that if I repeated the dependencies in the type checks that the types appear to be checked, but that requires repeating the dependencies from setup.cfg (and thus keeping them in sync). e.g.:

twine feature/importlib-metadata-entry-points-select $ git diff
diff --git a/mypy.ini b/mypy.ini
index dc7d662..ddc192f 100644
--- a/mypy.ini
+++ b/mypy.ini
@@ -22,10 +22,6 @@ strict_equality = True
 ; https://github.com/tartley/colorama/issues/206
 ignore_missing_imports = True
 
-[mypy-importlib_metadata]
-; https://github.com/python/importlib_metadata/issues/10
-ignore_missing_imports = True
-
 [mypy-keyring]
 ; https://github.com/jaraco/keyring/issues/437
 ignore_missing_imports = True
diff --git a/tox.ini b/tox.ini
index ae3bc0e..bf5d3ce 100644
--- a/tox.ini
+++ b/tox.ini
@@ -73,6 +73,7 @@ skip_install = True
 deps =
     mypy
     lxml
+    importlib_metadata >= 3.6
 commands =
     mypy --html-report mypy --txt-report mypy {posargs:twine}
     python -c 'with open("mypy/index.txt") as f: print(f.read())'

Is that the intention, to keep a separate subset of dependencies in the 'types' check?

In my other projects, I enable ignore_missing_imports globally and let dependencies declare and implement typing support at their pace. Any reason not to do that?

[jaraco]

In #734, I tried to address the issue, but found I couldn't effectively suppress the failing type checks, so I'm giving up.

[bhrutledge]

@jaraco Thanks for pointing out an opportunity to improve the type-checking process; I don't pretend that I got it right, and I'm always interested in learning more about the tooling.

Can you elaborate on "(a) not installing them"? Is that the because of the skip_install?

twine/tox.ini

Lines 71 to 72 in c1807fa

	[testenv:types]
	skip_install = True

If we remove that, will that resolve the issue with dependencies not being type-checked, and avoid the need to duplicate them in deps? I'm happy to experiment with this.

Re: not enabling ignore_missing_imports globally, here's my original rationale, from #614:

Use explicit ignore_missing_imports. I think this could help us keep an eye on projects that might add annotations in the future. It will also cause mypy to fail if somebody adds a library that's missing them, which seems like an opportunity to make a thoughtful choice about how to proceed.

[jaraco]

Good point separating the two concerns. Yes, removing skip_install will bring in the dependencies. I think you're right about the second point.

[jaraco]

Yesterday, in keyring, I added this change, which if I understand correctly should solve the failing type checks:

twine/auth.py:58: error: Call to untyped function "get_credential" in typed context
twine/auth.py:70: error: Call to untyped function "get_password" in typed context

But it didn't.

I'm surprised that mypy couldn't resolve the types for get_credential and get_password, because get_keyring resolves to a KeyringBackend, whose get_credential and get_password both declare return types.

In this change, I even added types to the parameters for get_password and get_credential, but twine emits the same error.

I wonder if the problem is that the accessor functions don't have type declarations. I would have expected mypy to infer the parameter and return types for those accessor functions from the types declared on the delegated behavior internally. I'm disinterested in copy/pasting types through every accessor function, and I'm not even sure that would help.

[jaraco]

In jaraco/keyring@ceeaa92, I added the redundant type definitions I was reluctant to add. With these, twine now complains thus:

twine/auth.py:58: error: Argument 1 to "get_credential" has incompatible type "Optional[str]"; expected "str"
twine/auth.py:70: error: Argument 1 to "get_password" has incompatible type "Optional[str]"; expected "str"
twine/auth.py:70: error: Argument 2 to "get_password" has incompatible type "_lru_cache_wrapper[Optional[str]]"; expected "str"

[jaraco]

It seems that twine's aggressive typing settings, and in this case disallow_untyped_calls = True, is what's enforcing the need for downstream libraries to aggressively and redundantly type their functions.

[jaraco]

I filed an issue upstream with mypy to see if there's some way the inference could be improved. In the meantime, I'll cut a new release of keyring to satisfy twine's usage. I've also found that twine's usage needs some tweaks to support this new mode, so I've applied those in 4cd95a6.

[bhrutledge]

Thanks, @jaraco. I didn't dive into all the links you provided, but IIRC the various ways that username/password can be sourced led to some typing complexity.

It seems that twine's aggressive typing settings, and in this case disallow_untyped_calls = True, is what's enforcing the need for downstream libraries to aggressively and redundantly type their functions.

FWIW, the Twine typing configuration is the equivalent of --strict. Are you suggesting a change to that?

[jaraco]

No. At this point, I'm not suggesting any change. I'm leaning toward reducing strictness, because the amount of time I'm spending fussing with types continues to increase while the value I get from them remains small. At this point, I'll persevere through. If it remains untenable, I'll write up another proposal. Thanks for asking.

[sigmavirus24]

while the value I get from them remains small

Yet the value it provides to others seems to be immense. It sounds like you're suggesting if this continues to just inconvenience you, you'll undo the hard work of others. I don't think that's actually what you're trying to suggest, but that's what it comes across as