Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add minimal 'mypy' run to the pre-commit hooks. #4176

Merged
merged 2 commits into from
Jun 20, 2020
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
11 changes: 11 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -182,3 +182,14 @@
aiida/__init__.py
)$
pass_filenames: false

- repo: https://github.com/pre-commit/mirrors-mypy
rev: v0.780
hooks:
- id: mypy
files: >-
(?x)^(
aiida/engine/processes/calcjobs/calcjob.py|
aiida/tools/groups/paths.py
)$
pass_filenames: true
82 changes: 29 additions & 53 deletions aiida/tools/groups/paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from collections import namedtuple
from functools import total_ordering
import re
from typing import Any, Iterable, List, Optional # pylint: disable=unused-import
from typing import Any, Iterator, List, Optional, Tuple
import warnings

from aiida import orm
Expand Down Expand Up @@ -60,8 +60,7 @@ class GroupPath:
See tests for usage examples.
"""

def __init__(self, path='', cls=orm.Group, warn_invalid_child=True):
# type: (str, Optional[str], Optional[GroupPath])
def __init__(self, path: str = '', cls: orm.groups.GroupMeta = orm.Group, warn_invalid_child: bool = True) -> None:
"""Instantiate the class.

:param path: The initial path of the group.
Expand All @@ -88,69 +87,59 @@ def _validate_path(self, path):
raise InvalidPath("The path may not start/end with the delimiter '{}': {}".format(self._delimiter, path))
return path

def __repr__(self):
# type: () -> str
def __repr__(self) -> str:
"""Represent the instantiated class."""
return "{}('{}', cls='{}')".format(self.__class__.__name__, self.path, self.cls)

def __eq__(self, other):
# type: (Any) -> bool
def __eq__(self, other: Any) -> bool:
"""Compare equality of path and ``Group`` subclass to another ``GroupPath`` object."""
if not isinstance(other, GroupPath):
return NotImplemented
return (self.path, self.cls) == (other.path, other.cls)

def __lt__(self, other):
# type: (Any) -> bool
def __lt__(self, other: Any) -> bool:
"""Compare less-than operator of path and ``Group`` subclass to another ``GroupPath`` object."""
if not isinstance(other, GroupPath):
return NotImplemented
return (self.path, self.cls) < (other.path, other.cls)

@property
def path(self):
# type: () -> str
def path(self) -> str:
"""Return the path string."""
return self._path_string

@property
def path_list(self):
# type: () -> List[str]
def path_list(self) -> List[str]:
"""Return a list of the path components."""
return self._path_list[:]

@property
def key(self):
# type: () -> str
def key(self) -> Optional[str]:
"""Return the final component of the the path."""
if self._path_list:
return self._path_list[-1]
return None

@property
def delimiter(self):
# type: () -> str
def delimiter(self) -> str:
"""Return the delimiter used to split path into components."""
return self._delimiter

@property
def cls(self):
# type: () -> str
def cls(self) -> orm.groups.GroupMeta:
"""Return the cls used to query for and instantiate a ``Group`` with."""
return self._cls

@property
def parent(self):
# type: () -> Optional[GroupPath]
def parent(self) -> Optional['GroupPath']:
"""Return the parent path."""
if self.path_list:
return GroupPath(
self.delimiter.join(self.path_list[:-1]), cls=self.cls, warn_invalid_child=self._warn_invalid_child
)
return None

def __truediv__(self, path):
# type: (str) -> GroupPath
def __truediv__(self, path: str) -> 'GroupPath':
"""Return a child ``GroupPath``, with a new path formed by appending ``path`` to the current path."""
if not isinstance(path, str):
raise TypeError('path is not a string: {}'.format(path))
Expand All @@ -162,22 +151,19 @@ def __truediv__(self, path):
)
return child

def __getitem__(self, path):
# type: (str) -> GroupPath
def __getitem__(self, path: str) -> 'GroupPath':
"""Return a child ``GroupPath``, with a new path formed by appending ``path`` to the current path."""
return self.__truediv__(path)

def get_group(self):
# type: () -> Optional[self.cls]
def get_group(self) -> Optional['GroupPath']:
"""Return the concrete group associated with this path."""
try:
return orm.QueryBuilder().append(self.cls, subclassing=False, filters={'label': self.path}).one()[0]
except NotExistent:
return None

@property
def group_ids(self):
# type: () -> List[int]
def group_ids(self) -> List[int]:
"""Return all the UUID associated with this GroupPath.

:returns: and empty list, if no group associated with this label,
Expand All @@ -192,13 +178,11 @@ def group_ids(self):
return query.all(flat=True)

@property
def is_virtual(self):
# type: () -> bool
def is_virtual(self) -> bool:
"""Return whether there is one or more concrete groups associated with this path."""
return len(self.group_ids) == 0

def get_or_create_group(self):
# type: () -> (self.cls, bool)
def get_or_create_group(self) -> Tuple[orm.Group, bool]:
"""Return the concrete group associated with this path or, create it, if it does not already exist."""
return self.cls.objects.get_or_create(label=self.path)

Expand All @@ -215,8 +199,7 @@ def delete_group(self):
self.cls.objects.delete(ids[0])

@property
def children(self):
# type: () -> Iterable[GroupPath]
def children(self) -> Iterator['GroupPath']:
"""Iterate through all (direct) children of this path."""
query = orm.QueryBuilder()
filters = {}
Expand All @@ -240,26 +223,22 @@ def children(self):
if self._warn_invalid_child:
warnings.warn('invalid path encountered: {}'.format(path_string)) # pylint: disable=no-member

def __iter__(self):
# type: () -> Iterable[GroupPath]
def __iter__(self) -> Iterator['GroupPath']:
"""Iterate through all (direct) children of this path."""
return self.children

def __len__(self):
# type: () -> int
def __len__(self) -> int:
"""Return the number of children for this path."""
return sum(1 for _ in self.children)

def __contains__(self, key):
# type: (str) -> bool
def __contains__(self, key: str) -> bool:
"""Return whether a child exists for this key."""
for child in self.children:
if child.path_list[-1] == key:
return True
return False

def walk(self, return_virtual=True):
# type: () -> Iterable[GroupPath]
def walk(self, return_virtual: bool = True) -> Iterator['GroupPath']:
"""Recursively iterate through all children of this path."""
for child in self:
if return_virtual or not child.is_virtual:
Expand All @@ -268,8 +247,9 @@ def walk(self, return_virtual=True):
if return_virtual or not sub_child.is_virtual:
yield sub_child

def walk_nodes(self, filters=None, node_class=None, query_batch=None):
# type: () -> Iterable[WalkNodeResult]
def walk_nodes(
self, filters: Optional[dict] = None, node_class: Optional[orm.Node] = None, query_batch: Optional[int] = None
) -> Iterator[WalkNodeResult]:
"""Recursively iterate through all nodes of this path and its children.

:param filters: filters to apply to the node query
Expand Down Expand Up @@ -311,27 +291,23 @@ class GroupAttr:

"""

def __init__(self, group_path):
# type: (GroupPath)
def __init__(self, group_path: GroupPath) -> None:
"""Instantiate the ``GroupPath``, and a mapping of its children."""
self._group_path = group_path

def __repr__(self):
# type: () -> str
def __repr__(self) -> str:
"""Represent the instantiated class."""
return "{}('{}', type='{}')".format(self.__class__.__name__, self._group_path.path, self._group_path.cls)

def __call__(self):
# type: () -> GroupPath
def __call__(self) -> GroupPath:
"""Return the ``GroupPath``."""
return self._group_path

def __dir__(self):
"""Return a list of available attributes."""
return [c.path_list[-1] for c in self._group_path.children if REGEX_ATTR.match(c.path_list[-1])]

def __getattr__(self, attr):
# type: (str) -> GroupAttr
def __getattr__(self, attr) -> 'GroupAttr':
"""Return the requested attribute name."""
for child in self._group_path.children:
if attr == child.path_list[-1]:
Expand Down
41 changes: 41 additions & 0 deletions mypy.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Global options

[mypy]
python_version = 3.5

follow_imports = skip

; Strictness settings
; disallow_any_unimported = True
; disallow_any_expr = True
; disallow_any_decorated = True
; disallow_any_explicit = True
; disallow_any_generics = True
; disallow_subclassing_any = True

; disallow_untyped_calls = True
; disallow_untyped_defs = True
; disallow_incomplete_defs = True
; disallow_untyped_decorators = True

; no_implicit_optional = True
; no_strict_optional = False

; Enable all warnings
; warn_redundant_casts = True
; warn_unused_ignores = True
; warn_return_any = True
; warn_unreachable = True

; allow_untyped_globals = False
; strict_equality = True
Comment on lines +8 to +31
Copy link
Contributor

Choose a reason for hiding this comment

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

can we get rid of all these if we are not using them?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, although it might be nice seeing them listed here since (I think) the goal would be to slowly enable some of them.


[mypy-numpy.*]
ignore_missing_imports = True

[mypy-plumpy.*]
ignore_missing_imports = True
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this similar to pylint issuing missing import warnings if it is not being run in the "system" environment? If so, would it make sense to also run mypy under system?

Copy link
Member Author

Choose a reason for hiding this comment

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

Mypy.. has its own "import system" - the problem here is that when dependencies don't have type annotations, that will be reported as a failure.

For now I'm not sure this makes a difference since we also use skip for the follow_imports option, but we definitely want to get rid of that one eventually.

The import mechanism is described here: https://mypy.readthedocs.io/en/stable/running_mypy.html#how-mypy-handles-imports


[mypy-scipy.*]
ignore_missing_imports = True