Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ repos:
- id: mypy
name: mypy
entry: mypy
args: [--config-file=pyproject.toml]
args: [--config-file=pyproject.toml, --pretty]
language: python
types: [python]
require_serial: true
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ notebook = [
]
pre-commit = [
'aiida-core[atomic_tools,rest,tests,tui]',
'mypy~=1.13.0',
'mypy~=1.16.0',
'packaging~=23.0',
'pre-commit~=3.5',
'sqlalchemy[mypy]~=2.0',
Expand Down
4 changes: 2 additions & 2 deletions src/aiida/cmdline/commands/cmd_profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ def profile_setup():
"""Set up a new profile."""


@verdi_profile.command('configure-rabbitmq') # type: ignore[arg-type]
@verdi_profile.command('configure-rabbitmq')
@arguments.PROFILE(default=defaults.get_default_profile)
@options.FORCE()
@setup.SETUP_BROKER_PROTOCOL()
Expand All @@ -142,7 +142,7 @@ def profile_setup():
@setup.SETUP_BROKER_VIRTUAL_HOST()
@options.NON_INTERACTIVE(default=True, show_default='--non-interactive')
@click.pass_context
def profile_configure_rabbitmq(ctx, profile, non_interactive, force, **kwargs):
def profile_configure_rabbitmq(ctx, /, profile, non_interactive, force, **kwargs):
"""Configure RabbitMQ for a profile.

Enable RabbitMQ for a profile that was created without a broker, or reconfigure existing connection details.
Expand Down
4 changes: 2 additions & 2 deletions src/aiida/cmdline/groups/dynamic.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def __init__(
command: t.Callable,
entry_point_group: str,
entry_point_name_filter: str = r'.*',
shared_options: list[click.Option] | None = None,
shared_options: list[t.Callable[[t.Any], t.Any]] | None = None,
**kwargs,
):
super().__init__(**kwargs)
Expand Down Expand Up @@ -143,7 +143,7 @@ def apply_options(func):

return apply_options

def list_options(self, entry_point: str) -> list:
def list_options(self, entry_point: str) -> list[t.Callable[[t.Any], t.Any]]:
"""Return the list of options that should be applied to the command for the given entry point.

:param entry_point: The entry point.
Expand Down
28 changes: 16 additions & 12 deletions src/aiida/engine/processes/functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -355,11 +355,11 @@
"""
if (
not issubclass(node_class, ProcessNode) # type: ignore[redundant-expr]
or not issubclass(node_class, FunctionCalculationMixin) # type: ignore[unreachable]
or not issubclass(node_class, FunctionCalculationMixin)
):
raise TypeError('the node_class should be a sub class of `ProcessNode` and `FunctionCalculationMixin`')

signature = inspect.signature(func) # type: ignore[unreachable]
signature = inspect.signature(func)

Check warning on line 362 in src/aiida/engine/processes/functions.py

View check run for this annotation

Codecov / codecov/patch

src/aiida/engine/processes/functions.py#L362

Added line #L362 was not covered by tests

args: list[str] = []
var_positional: str | None = None
Expand All @@ -373,17 +373,21 @@
LOGGER.warning(f'function `{func.__name__}` has invalid type hints: {exception}')
annotations = {}

try:
parsed_docstring = docstring_parser.parse(func.__doc__)
except Exception as exception:
LOGGER.warning(f'function `{func.__name__}` has a docstring that could not be parsed: {exception}')
param_help_string = {}
if func.__doc__ is None:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes:

src/aiida/engine/processes/functions.py:377: error: Argument 1 to "parse" has incompatible type "str | None"; expected "str"  [arg-type]

Github is very confused, the diff here is actually not that bad, we're just handling the func.__doc__ is None case pre-emptively. (We could also log a warning if a calcfunction is missing a docstring, but that seems overly pedantic)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also log a warning if a calcfunction is missing a docstring, but that seems overly pedantic

Yes, I think that is a bit too much, I won't throw warning for that.

param_help_string: dict[str, str | None] = {}

Check warning on line 377 in src/aiida/engine/processes/functions.py

View check run for this annotation

Codecov / codecov/patch

src/aiida/engine/processes/functions.py#L376-L377

Added lines #L376 - L377 were not covered by tests
namespace_help_string = None
else:
param_help_string = {param.arg_name: param.description for param in parsed_docstring.params}
namespace_help_string = parsed_docstring.short_description if parsed_docstring.short_description else ''
if parsed_docstring.long_description is not None:
namespace_help_string += f'\n\n{parsed_docstring.long_description}'
try:
parsed_docstring = docstring_parser.parse(func.__doc__)
except Exception as exception:
LOGGER.warning(f'function `{func.__name__}` has a docstring that could not be parsed: {exception}')
param_help_string = {}
namespace_help_string = None

Check warning on line 385 in src/aiida/engine/processes/functions.py

View check run for this annotation

Codecov / codecov/patch

src/aiida/engine/processes/functions.py#L380-L385

Added lines #L380 - L385 were not covered by tests
else:
param_help_string = {param.arg_name: param.description for param in parsed_docstring.params}
namespace_help_string = parsed_docstring.short_description if parsed_docstring.short_description else ''
if parsed_docstring.long_description is not None:
namespace_help_string += f'\n\n{parsed_docstring.long_description}'

Check warning on line 390 in src/aiida/engine/processes/functions.py

View check run for this annotation

Codecov / codecov/patch

src/aiida/engine/processes/functions.py#L387-L390

Added lines #L387 - L390 were not covered by tests

for key, parameter in signature.parameters.items():
if parameter.kind in [parameter.POSITIONAL_ONLY, parameter.POSITIONAL_OR_KEYWORD, parameter.KEYWORD_ONLY]:
Expand Down Expand Up @@ -435,7 +439,7 @@
def indirect_default(value=default):
return to_aiida_type(value)
else:
indirect_default = default
indirect_default = default # type: ignore[assignment]

Check warning on line 442 in src/aiida/engine/processes/functions.py

View check run for this annotation

Codecov / codecov/patch

src/aiida/engine/processes/functions.py#L442

Added line #L442 was not covered by tests
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not super clear how to solve this one, since indirect_default can be basically anything

src/aiida/engine/processes/functions.py:438: error: Incompatible types in assignment (expression has type "Any | tuple[()]", variable has type "Callable[[Any], Any]") [assignment]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it can be anything then as error message suggest use Callable[[Any], Any] ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what are you suggesting here? the callable type is inferred because indirect_default is defined as a function in the if branch above, but in the else branch in can be anything. I think this we could squash this by inverting the logic and put the else branch first, but I don't really want to touch the logic.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I misunderstood the change here.

I think the mypy complain because the typechecker resolve the expression in if..else.. branch and want to make sure the type of expression conform with each other.

Then the correct way to solve this might be for the else branch do:

def indirect_default(value=default):
    return lambda _: value

(I am not sure this is correct, but I think it is better to just ignore the type error here to not touch the logic. so if you don't want to try please ignore my comment.)


spec.input(
parameter.name,
Expand Down
4 changes: 2 additions & 2 deletions src/aiida/engine/processes/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
class SaveKeys(enum.Enum):
"""Keys used to identify things in the saved instance state bundle."""

CALC_ID: str = 'calc_id'
CALC_ID = 'calc_id'

@classmethod
def spec(cls) -> ProcessSpec:
Expand Down Expand Up @@ -379,7 +379,7 @@
continue
try:
result = self.runner.controller.kill_process(child.pk, msg_text=f'Killed by parent<{self.node.pk}>')
result = asyncio.wrap_future(result) # type: ignore[arg-type]
result = asyncio.wrap_future(result)

Check warning on line 382 in src/aiida/engine/processes/process.py

View check run for this annotation

Codecov / codecov/patch

src/aiida/engine/processes/process.py#L382

Added line #L382 was not covered by tests
if asyncio.isfuture(result):
killing.append(result)
except ConnectionClosed:
Expand Down
9 changes: 5 additions & 4 deletions src/aiida/orm/nodes/data/code/installed.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from __future__ import annotations

import pathlib
from typing import cast

from pydantic import field_serializer, field_validator

Expand Down Expand Up @@ -47,15 +48,15 @@
...,
title='Computer',
description='The remote computer on which the executable resides.',
orm_to_model=lambda node, _: node.computer.label,
orm_to_model=lambda node, _: cast('InstalledCode', node).computer.label,
short_name='-Y',
priority=2,
)
filepath_executable: str = MetadataField(
...,
title='Filepath executable',
description='Filepath of the executable on the remote computer.',
orm_to_model=lambda node, _: str(node.filepath_executable), # type: ignore[attr-defined]
orm_to_model=lambda node, _: str(cast('InstalledCode', node).filepath_executable),
short_name='-X',
priority=1,
)
Expand Down Expand Up @@ -83,7 +84,7 @@
"""
super().__init__(**kwargs)
self.computer = computer
self.filepath_executable = filepath_executable # type: ignore[assignment]
self.filepath_executable = filepath_executable

Check warning on line 87 in src/aiida/orm/nodes/data/code/installed.py

View check run for this annotation

Codecov / codecov/patch

src/aiida/orm/nodes/data/code/installed.py#L87

Added line #L87 was not covered by tests

def _validate(self):
"""Validate the instance by checking that a computer has been defined.
Expand Down Expand Up @@ -159,7 +160,7 @@
"""
return self.filepath_executable

@property # type: ignore[override]
@property
def computer(self) -> Computer:
"""Return the computer of this code."""
assert self.backend_entity.computer is not None
Expand Down
2 changes: 1 addition & 1 deletion src/aiida/orm/nodes/data/code/legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ def get_code_helper(cls, label, machinename=None, backend=None):
return result[0]

@classmethod
def get(cls, pk=None, label=None, machinename=None):
def get(cls, pk=None, label=None, machinename=None): # type: ignore[override]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why mypy complains here:

src/aiida/orm/nodes/data/code/legacy.py:234: error: Signature of "get"
incompatible with supertype "Entity"  [override]
        def get(cls, pk=None, label=None, machinename=None):
        ^
src/aiida/orm/nodes/data/code/legacy.py:234: note:      Superclass:
src/aiida/orm/nodes/data/code/legacy.py:234: note:          @classmethod
src/aiida/orm/nodes/data/code/legacy.py:234: note:          def get(cls, **kwargs: Any) -> Any
src/aiida/orm/nodes/data/code/legacy.py:234: note:      Subclass:
src/aiida/orm/nodes/data/code/legacy.py:234: note:          @classmethod
src/aiida/orm/nodes/data/code/legacy.py:234: note:          def get(cls, pk: Any = ..., label: Any = ..., machinename: Any = ...) -> Any

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you asked, my answer is because we did bad design by keep on doing too much bad inheritance.
I'd just add an ignore, otherwise need to add an other function signature dispatch or change the signature of parent function which can cause unexpect things.

"""Get a Computer object with given identifier string, that can either be
the numeric ID (pk), or the label (and computername) (if unique).

Expand Down
2 changes: 1 addition & 1 deletion src/aiida/orm/nodes/data/code/portable.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@
if not filepath_files_path.is_dir():
raise ValueError(f'The filepath `{filepath_files}` is not a directory.')

self.filepath_executable = filepath_executable # type: ignore[assignment] # should be fixed in mypy 1.15 see mypy/commit/1eb9d4c
self.filepath_executable = filepath_executable

Check warning on line 107 in src/aiida/orm/nodes/data/code/portable.py

View check run for this annotation

Codecov / codecov/patch

src/aiida/orm/nodes/data/code/portable.py#L107

Added line #L107 was not covered by tests
self.base.repository.put_object_from_tree(str(filepath_files))

def _validate(self):
Expand Down
2 changes: 1 addition & 1 deletion src/aiida/orm/nodes/process/calculation/calcfunction.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def validate_outgoing(self, target: 'Node', link_type: LinkType, link_label: str
)


class CalcFunctionNode(FunctionCalculationMixin, CalculationNode): # type: ignore[misc]
class CalcFunctionNode(FunctionCalculationMixin, CalculationNode):
"""ORM class for all nodes representing the execution of a calcfunction."""

_CLS_NODE_LINKS = CalcFunctionNodeLinks
2 changes: 1 addition & 1 deletion src/aiida/orm/nodes/process/workflow/workchain.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class WorkChainNode(WorkflowNode):
STEPPER_STATE_INFO_KEY = 'stepper_state_info'

@classproperty
def _updatable_attributes(cls) -> Tuple[str, ...]: # type: ignore[override] # noqa: N805
def _updatable_attributes(cls) -> Tuple[str, ...]: # noqa: N805
return super()._updatable_attributes + (cls.STEPPER_STATE_INFO_KEY,)

@property
Expand Down
2 changes: 1 addition & 1 deletion src/aiida/orm/nodes/process/workflow/workfunction.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def validate_outgoing(self, target: 'Node', link_type: LinkType, link_label: str
)


class WorkFunctionNode(FunctionCalculationMixin, WorkflowNode): # type: ignore[misc]
class WorkFunctionNode(FunctionCalculationMixin, WorkflowNode):
"""ORM class for all nodes representing the execution of a workfunction."""

_CLS_NODE_LINKS = WorkFunctionNodeLinks
5 changes: 1 addition & 4 deletions src/aiida/repository/backend/abstract.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,7 @@
:return: the generated fully qualified identifier for the object within the repository.
:raises TypeError: if the handle is not a byte stream.
"""
if (
not isinstance(handle, io.BufferedIOBase) # type: ignore[redundant-expr,unreachable]
and not self.is_readable_byte_stream(handle)
):
if not isinstance(handle, io.BufferedIOBase) and not self.is_readable_byte_stream(handle):

Check warning on line 78 in src/aiida/repository/backend/abstract.py

View check run for this annotation

Codecov / codecov/patch

src/aiida/repository/backend/abstract.py#L78

Added line #L78 was not covered by tests
raise TypeError(f'handle does not seem to be a byte stream: {type(handle)}.')
return self._put_object_from_filelike(handle)

Expand Down
2 changes: 1 addition & 1 deletion src/aiida/tools/archive/imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,7 @@
raise ImportUniquenessError(
f'Node {in_id} already has an outgoing {link_type.value!r} link with label {link_label!r}'
)
if 'out_id' in link_uniqueness and out_id in existing_out_id_label:
if 'out_id' in link_uniqueness and out_id in existing_out_id:

Check warning on line 952 in src/aiida/tools/archive/imports.py

View check run for this annotation

Codecov / codecov/patch

src/aiida/tools/archive/imports.py#L952

Added line #L952 was not covered by tests
raise ImportUniquenessError(f'Node {out_id} already has an incoming {link_type.value!r} link')
if 'out_id_label' in link_uniqueness and (out_id, link_label) in existing_out_id_label:
raise ImportUniquenessError(
Expand Down
Loading
Loading