More typing for cmdline.params module #7009

danielhollas · 2025-09-17T22:56:24Z

I ran mypy in strict mode on the aiida.cmdline.params module and fixed all issues (mostly untyped functions). Then I enabled stricter settings for this module in pyproject.toml

Extra safety before the click 8.2 update (to which I've become paranoid again 😅 )

Also upgraded mypy to 1.18.1. Changelog promises big performance speedup in this release, and this is what I also see locally! 28s -> 15s without cache and 4.5s -> 0.5 with cache! 🎉
(also removed one type: ignore)

Mypy==1.17.1

$ rm -rf .mypy_cache
$ time pre-commit run -a mypy
real	0m28.563s
user	0m27.509s
sys	0m0.880s
$ time pre-commit run -a mypy
real	0m4.629s
user	0m4.176s
sys	0m0.426s

Mypy==1.18.1

$ rm -rf .mypy_cache
$ time pre-commit run -a mypy
real	0m14.716s
user	0m13.714s
sys	0m0.910s
$ time pre-commit run -a mypy
real	0m0.483s
user 0m0.335s
sys	0m0.147s

codecov · 2025-09-17T23:18:14Z

Codecov Report

❌ Patch coverage is 87.20379% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.03%. Comparing base (7255f01) to head (ef114af).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/aiida/cmdline/params/types/path.py	78.95%	4 Missing ⚠️
.../aiida/cmdline/params/options/commands/computer.py	70.00%	3 Missing ⚠️
src/aiida/cmdline/params/types/config.py	75.00%	2 Missing ⚠️
src/aiida/cmdline/params/types/user.py	77.78%	2 Missing ⚠️
src/aiida/cmdline/groups/dynamic.py	85.72%	1 Missing ⚠️
src/aiida/cmdline/params/arguments/overridable.py	83.34%	1 Missing ⚠️
src/aiida/cmdline/params/options/conditional.py	85.72%	1 Missing ⚠️
src/aiida/cmdline/params/options/config.py	85.72%	1 Missing ⚠️
src/aiida/cmdline/params/options/interactive.py	87.50%	1 Missing ⚠️
src/aiida/cmdline/params/options/main.py	75.00%	1 Missing ⚠️
... and 10 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7009      +/-   ##
==========================================
- Coverage   79.04%   79.03%   -0.01%     
==========================================
  Files         566      566              
  Lines       43720    43798      +78     
==========================================
+ Hits        34555    34612      +57     
- Misses       9165     9186      +21

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

danielhollas · 2025-09-18T16:20:34Z

src/aiida/cmdline/params/arguments/overridable.py

 import click

+if t.TYPE_CHECKING:
+    from click.decorators import FC


FC is a TypeVar (type parameter) defined here:

https://github.com/pallets/click/blob/2a0e3ba907927ade6951d5732b775f11b54cb766/src/click/decorators.py#L25

It's not clear whether it's better to import it or redefine it here (since this is a type-checking only import I think it is fine, but I don't know what is a best practice in these cases).

I'd say import is fine. Better than to re-define!

danielhollas · 2025-09-18T16:24:27Z

src/aiida/cmdline/params/types/strings.py

-            return None
+    def convert(self, value: t.Any, param: click.Parameter | None, ctx: click.Context | None) -> str:
+        # NOTE: The return value of click.StringParamType.convert is typed as t.Any,
+        # but from its implementation its clear that it returns a string.


https://github.com/pallets/click/blob/2a0e3ba907927ade6951d5732b775f11b54cb766/src/click/types.py#L210

danielhollas · 2025-09-18T16:26:48Z

src/aiida/cmdline/params/types/strings.py

-    def convert(self, value, param, ctx):
-        newval = super().convert(value, param, ctx)
-
-        # Note: Valid :py:class:`click.ParamType`s need to pass through None unchanged


This was perhaps true in click 7.x but doesn't seem to be true anymore; see docstring of ParamType.convert() method:

https://github.com/pallets/click/blob/2a0e3ba907927ade6951d5732b775f11b54cb766/src/click/types.py#L105

And also the code here that calls the convert method only when the value is not None.
https://github.com/pallets/click/blob/2a0e3ba907927ade6951d5732b775f11b54cb766/src/click/types.py#L89

danielhollas · 2025-09-18T20:32:27Z

src/aiida/orm/implementation/entities.py

    """Container class that represents a collection of entries of a particular backend entity."""

-    ENTITY_CLASS: ClassVar[Type[EntityType]]  # type: ignore[misc]
+    ENTITY_CLASS: ClassVar[Type[EntityType]]


ignore not needed thanks to mypy upgrade 🎉

GeigerJ2

If the PR passes strict mypy checking, chances are high that it also passes my review :3

GeigerJ2 · 2025-09-22T14:48:09Z

src/aiida/cmdline/params/arguments/overridable.py

 import click

+if t.TYPE_CHECKING:
+    from click.decorators import FC


I'd say import is fine. Better than to re-define!

GeigerJ2 · 2025-09-22T15:44:45Z

src/aiida/cmdline/params/options/commands/computer.py

    if job_resource_cls is None:
        # Odd situation...
-        return False
+        return False  # type: ignore[unreachable]


Can probably be removed in the future...

GeigerJ2 · 2025-09-24T08:42:05Z

src/aiida/cmdline/params/types/path.py

+if t.TYPE_CHECKING:
+    try:
+        from typing import TypeAlias
+    except ImportError:
+        from typing_extensions import TypeAlias


Just a reminder that we can remove this once we drop py 3.9

GeigerJ2 · 2025-09-24T09:15:32Z

src/aiida/common/lang.py

+T = TypeVar('T')
+
+
+def type_check(what: T, of_type: Any, msg: 'str | None' = None, allow_none: bool = False) -> 'T | None':


Any reason for the forward reference quotes here instead of adding from future import annotations?

GeigerJ2 · 2025-09-24T09:21:18Z

tests/cmdline/params/types/test_path.py


 def test_fail_unreachable_url():
    """Test the parameter in case of a valid URL that cannot be reached."""
+    # TODO: Mock the request to make this faster


waiting for the PR hehe

GeigerJ2 · 2025-09-24T09:23:55Z

pyproject.toml

 pre-commit = [
  'aiida-core[atomic_tools,rest,tests,tui]',
-  'mypy~=1.17.0',
+  'mypy~=1.18.0',


Do that in a separate PR? Or fine to do it alongside this one? Not sure what is best practice here...

I proclaim that because there was a no fallout from this upgrade it's okay to do it here :-)

danielhollas requested a review from GeigerJ2 September 17, 2025 22:56

danielhollas added the topic/typing label Sep 17, 2025

More typing for cmdline.params.{arguments,types} modules

3c14634

danielhollas force-pushed the params-types-typing branch from 15c22ce to 3c14634 Compare September 17, 2025 23:07

danielhollas added 2 commits September 18, 2025 00:15

More precise Callable type

c7ea870

Fix

6060dc7

danielhollas added 2 commits September 18, 2025 12:38

Fix docs build

0acbb3d

Better types for cmdline.params.types.strings module

c047aac

danielhollas commented Sep 18, 2025

View reviewed changes

danielhollas added 3 commits September 18, 2025 18:00

More accurate return types of convert methods

359461d

Strict typing for cmdline.params.options

9e322a8

Don't ignore errors from plumpy

1d005c7

danielhollas requested review from unkcpz and agoscinski as code owners September 18, 2025 19:03

danielhollas added 4 commits September 18, 2025 20:45

Enable strict settings for aiida.cmdline.params

55b8947

Fixes

43e2aba

Upgrade mypy to 1.18.1

4754cc2

Nitpick

d7ab2bf

danielhollas commented Sep 18, 2025

View reviewed changes

danielhollas changed the title ~~More typing for cmdline.params.types module~~ More typing for cmdline.params module Sep 18, 2025

danielhollas added 2 commits September 23, 2025 20:12

Strict typing for cmdline.groups module

5b82fbf

Merge branch 'main' into params-types-typing

ef114af

GeigerJ2 approved these changes Sep 24, 2025

View reviewed changes

danielhollas merged commit 0d5c4d4 into aiidateam:main Sep 24, 2025
32 checks passed

danielhollas deleted the params-types-typing branch September 24, 2025 09:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

More typing for cmdline.params module #7009

More typing for cmdline.params module #7009

Uh oh!

danielhollas commented Sep 17, 2025 •

edited

Loading

Uh oh!

codecov bot commented Sep 17, 2025 •

edited

Loading

Uh oh!

danielhollas Sep 18, 2025

Uh oh!

GeigerJ2 Sep 22, 2025

Uh oh!

danielhollas Sep 18, 2025 •

edited

Loading

Uh oh!

danielhollas Sep 18, 2025

Uh oh!

danielhollas Sep 18, 2025

Uh oh!

GeigerJ2 left a comment

Uh oh!

GeigerJ2 Sep 22, 2025

Uh oh!

GeigerJ2 Sep 22, 2025

Uh oh!

GeigerJ2 Sep 24, 2025

Uh oh!

GeigerJ2 Sep 24, 2025

Uh oh!

GeigerJ2 Sep 24, 2025

Uh oh!

GeigerJ2 Sep 24, 2025

Uh oh!

danielhollas Sep 24, 2025

Uh oh!

Uh oh!

Uh oh!

		T = TypeVar('T')


		def type_check(what: T, of_type: Any, msg: 'str \| None' = None, allow_none: bool = False) -> 'T \| None':

More typing for cmdline.params module #7009

More typing for cmdline.params module #7009

Uh oh!

Conversation

danielhollas commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Mypy==1.17.1

Mypy==1.18.1

Uh oh!

codecov bot commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielhollas Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GeigerJ2 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

danielhollas commented Sep 17, 2025 •

edited

Loading

codecov bot commented Sep 17, 2025 •

edited

Loading

danielhollas Sep 18, 2025 •

edited

Loading