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 typing to signac.core.json. #313

Merged
merged 16 commits into from
Apr 2, 2020
Merged
Show file tree
Hide file tree
Changes from 15 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
25 changes: 15 additions & 10 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ orbs:
win: circleci/[email protected] # Enables Windows executors

jobs:
style-check:
pre-checks:
docker:
- image: circleci/python:3.8

Expand All @@ -19,18 +19,23 @@ jobs:
- run:
name: install-dependencies
command: |
pip install --progress-bar off --user -U flake8==3.7.1 pydocstyle==5.0.2
pip install --progress-bar off --user -U flake8==3.7.1 pydocstyle==5.0.2 mypy==0.770

- run:
name: style-check-code
name: check-style
command: |
python -m flake8 --show-source .

- run:
name: style-check-doc-strings
name: check-docstyle
command: |
pydocstyle

- run:
name: check-type-hints
command: |
mypy .

linux-python-38: &linux-template
docker:
- image: circleci/python:3.8
Expand Down Expand Up @@ -180,25 +185,25 @@ workflows:
version: 2
test:
jobs:
- style-check
- pre-checks
- linux-python-38:
requires:
- style-check
- pre-checks
- linux-python-37:
requires:
- style-check
- pre-checks
- linux-python-36:
requires:
- style-check
- pre-checks
- linux-python-35:
requires:
- style-check
- pre-checks
- linux-pypy-3:
requires:
- linux-python-36
- windows-python-38:
requires:
- style-check
- pre-checks
- check-metadata:
filters:
branches:
Expand Down
8 changes: 8 additions & 0 deletions changelog.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@ Version 1
next
----

Added
+++++

- Type annotations are validated during continuous integration (#313).

Fixed
+++++

- Fix the ``signac config verify`` command (previously broken) (#301, #302).

[1.4.0] -- 2020-02-28
Expand Down
3 changes: 3 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ exclude = configobj,passlib,cite.py,conf.py
match = jsondict.py
ignore = D105, D107, D203, D213

[mypy]
ignore_missing_imports = True
Copy link
Member Author

Choose a reason for hiding this comment

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

ignore_missing_imports is enabled so that mypy doesn't get mad about missing type annotations in dependencies like deprecation or numpy.


[coverage:run]
source = signac
omit =
Expand Down
4 changes: 0 additions & 4 deletions signac/common/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,5 @@ class ExportError(Error, RuntimeError):
pass


class FileNotFoundError(Error, FileNotFoundError):
Copy link
Member Author

Choose a reason for hiding this comment

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

This seemed like an unused compatibility layer for Python 2 (which did not have FileNotFoundError). I removed it.

pass


class FetchError(FileNotFoundError):
pass
2 changes: 1 addition & 1 deletion signac/common/host.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@


SESSION_PASSWORD_HASH_CACHE = SimpleKeyring()
SESSION_USERNAME_CACHE = dict()
SESSION_USERNAME_CACHE = dict() # type: ignore
Copy link
Member Author

Choose a reason for hiding this comment

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

Explanation: This requests a type annotation for the dict. I added ignores to all deprecated modules where necessary, without questioning it.


"""
THIS MODULE IS DEPRECATED!
Expand Down
2 changes: 1 addition & 1 deletion signac/contrib/filterparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def _cast(x):
"Attempt to interpret x with the correct type."
try:
if x in CAST_MAPPING_WARNING:
print("Did you mean {}?".format(CAST_MAPPING_WARNING[x], file=sys.stderr))
print("Did you mean {}?".format(CAST_MAPPING_WARNING[x]), file=sys.stderr)
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 caught a bug in the print statement! Fixed here.

return CAST_MAPPING[x]
except KeyError:
try:
Expand Down
2 changes: 1 addition & 1 deletion signac/contrib/indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ class RegexFileCrawler(BaseCrawler):
MyCrawler.define('.*\/a_(?P<a>\d+)\.txt', 'TextFile')
"""
"Mapping of compiled regex objects and associated formats."
definitions = dict()
definitions = dict() # type: ignore
Copy link
Member Author

Choose a reason for hiding this comment

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

Explanation: This requests a type annotation for the dict. I added ignores to all deprecated modules where necessary, without questioning it.


@classmethod
@deprecated(deprecated_in="1.3", removed_in="2.0", current_version=__version__,
Expand Down
2 changes: 1 addition & 1 deletion signac/contrib/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -1195,7 +1195,7 @@ def check(self):
if corrupted:
logger.error(
"At least one job appears to be corrupted. Call Project.repair() "
"to try to fix errors.".format(len(corrupted)))
"to try to fix errors.")
raise JobsCorruptedError(corrupted)

def repair(self, fn_statepoints=None, index=None, job_ids=None):
Expand Down
4 changes: 2 additions & 2 deletions signac/core/h5store.py
Original file line number Diff line number Diff line change
Expand Up @@ -507,8 +507,8 @@ class H5StoreManager(DictManager):
:param prefix:
The directory prefix shared by all stores managed by this class.
"""
cls = H5Store
suffix = '.h5'
cls = H5Store # type: ignore
suffix = '.h5' # type: ignore
Comment on lines +510 to +511
Copy link
Member Author

Choose a reason for hiding this comment

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

Explanation for the ignore: These are overriding a default of None in the DictManager parent class, meaning that the type is changing. That raises an error.


@staticmethod
def _validate_key(key):
Expand Down
5 changes: 3 additions & 2 deletions signac/core/json.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import logging
from json import load, loads, JSONEncoder
from json.decoder import JSONDecodeError
from typing import Any, Dict, Optional

logger = logging.getLogger(__name__)

Expand All @@ -21,7 +22,7 @@ class CustomJSONEncoder(JSONEncoder):
an object that is otherwise not serializable, by calling the object's
`_as_dict()` method.
"""
def default(self, o):
def default(self, o: Any) -> Dict[str, Any]:
if NUMPY:
if isinstance(o, numpy.number):
return o.item()
Expand All @@ -35,7 +36,7 @@ def default(self, o):
return super(CustomJSONEncoder, self).default(o)


def dumps(o, sort_keys=False, indent=None):
def dumps(o: Any, sort_keys: bool = False, indent: Optional[int] = None) -> str:
return CustomJSONEncoder(sort_keys=sort_keys, indent=indent).encode(o)


Expand Down
2 changes: 0 additions & 2 deletions signac/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
from .common.errors import ConfigError
from .common.errors import AuthenticationError
from .common.errors import ExportError
from .common.errors import FileNotFoundError
from .common.errors import FetchError

from .contrib.errors import DestinationExistsError
Expand Down Expand Up @@ -67,7 +66,6 @@ class KeyTypeError(TypeError):
'ConfigError',
'AuthenticationError',
'ExportError',
'FileNotFoundError',
'FetchError',
'DestinationExistsError',
'JobsCorruptedError',
Expand Down
8 changes: 4 additions & 4 deletions signac/syncutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@

logger = logging.getLogger('sync')
logging.addLevelName(LEVEL_MORE, 'MORE')
logging.MORE = LEVEL_MORE
logging.MORE = LEVEL_MORE # type: ignore
Copy link
Member Author

Choose a reason for hiding this comment

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

Explanation for the ignore: Assigning logging.MORE (and logger.more below) is setting a property that doesn't exist in the original module / class. This kind of monkey-patch raises an error:

signac/syncutil.py:14: error: Module has no attribute "MORE"
signac/syncutil.py:21: error: "Logger" has no attribute "more"



def log_more(msg, *args, **kwargs):
logger.log(LEVEL_MORE, msg, *args, **kwargs)


logger.more = log_more
logger.more = log_more # type: ignore


def copytree(src, dst, copy_function=shutil.copy2, symlinks=False):
Expand Down Expand Up @@ -48,13 +48,13 @@ def copytree(src, dst, copy_function=shutil.copy2, symlinks=False):


class dircmp_deep(dircmp):

def phase3(self): # Find out differences between common files
xx = filecmp.cmpfiles(self.left, self.right, self.common_files, shallow=False)
self.same_files, self.diff_files, self.funny_files = xx

methodmap = dict(dircmp.methodmap)
methodmap['samefiles'] = methodmap['diff_files'] = phase3
# The type check for the following line must be ignored: https://github.com/python/mypy/issues/708
bdice marked this conversation as resolved.
Show resolved Hide resolved
methodmap['same_files'] = methodmap['diff_files'] = phase3 # type: ignore


class _DocProxy(object):
Expand Down