add type hints to milc (now we'll never have a bug again!)

.gitignore

@@ -9,3 +9,4 @@ milc.egg-info
 venv
 site
 .venv
+.vscode

ci_tests

@@ -43,6 +43,12 @@ def main(cli):
         build_ok = False
         cli.log.error('Improperly formatted code. Please run this: yapf -i -r .')
 
+    cli.log.info('Running mypy...')
+    cmd = ['mypy', '--strict', 'milc']
+    result = run(cmd, stdin=DEVNULL)
+    if result.returncode != 0:
+        build_ok = False
+
     if build_ok:
         cli.log.info('{fg_green}All tests passed!')
         return True

milc/__init__.py

@@ -18,6 +18,7 @@
 import os
 import sys
 import warnings
+from typing import Optional
 
 from .emoji import EMOJI_LOGLEVELS
 from .milc import MILC
@@ -32,7 +33,13 @@
 cli = MILC()
 
 
-def set_metadata(*, name=None, author=None, version=None, logger=None):
+def set_metadata(
+    *,
+    name: Optional[str] = None,
+    author: Optional[str] = None,
+    version: Optional[str] = None,
+    logger: Optional[logging.Logger] = None,
+) -> MILC:
     """Set metadata about your program.
 
     This allows you to set the application's name, version, and/or author
@@ -48,6 +55,8 @@ def set_metadata(*, name=None, author=None, version=None, logger=None):
 
     cli = MILC(name, version, author, logger)
 
+    return cli
+
 
 # Extra stuff people can import
 from ._sparkline import sparkline  # noqa

milc/_in_argv.py

@@ -1,7 +1,8 @@
 import sys
+from typing import Optional
 
 
-def _in_argv(argument):
+def _in_argv(argument: str) -> bool:
     """Returns true if the argument is found is sys.argv.
 
     Since long options can be passed as either '--option value' or '--option=value' we need to check for both forms.
@@ -13,7 +14,7 @@ def _in_argv(argument):
     return False
 
 
-def _index_argv(argument):
+def _index_argv(argument: str) -> Optional[int]:
     """Returns the location of the argument in sys.argv, or None.
 
     Since long options can be passed as either '--option value' or '--option=value' we need to check for both forms.

milc/_sparkline.py

@@ -3,19 +3,35 @@
 """
 from decimal import Decimal
 from math import inf
+from typing import Any, List, Optional, TypeGuard
 
 from milc import cli
 
 spark_chars = '▁▂▃▄▅▆▇█'
 
 
-def is_number(i):
+def is_number(i: Any) -> TypeGuard[bool]:
     """Returns true if i is a number. Used to filter non-numbers from a list.
     """
     return isinstance(i, (int, float, Decimal))
 
 
-def sparkline(number_list, *, min_value=None, max_value=None, highlight_low=-inf, highlight_high=inf, highlight_low_color='', highlight_high_color='', negative_color='{fg_red}', positive_color='', highlight_low_reset='{fg_reset}', highlight_high_reset='{fg_reset}', negative_reset='{fg_reset}', positive_reset='{fg_reset}'):
+def sparkline(
+    number_list: List[Optional[int]],
+    *,
+    min_value: Optional[int] = None,
+    max_value: Optional[int] = None,
+    highlight_low: float = -inf,
+    highlight_high: float = inf,
+    highlight_low_color: str = '',
+    highlight_high_color: str = '',
+    negative_color: str = '{fg_red}',
+    positive_color: str = '',
+    highlight_low_reset: str = '{fg_reset}',
+    highlight_high_reset: str = '{fg_reset}',
+    negative_reset: str = '{fg_reset}',
+    positive_reset: str = '{fg_reset}',
+) -> str:
     """Display a sparkline from a sequence of numbers.
 
     If you wish to exclude extreme values, or you want to limit the set of characters used, you can adjust `min_value` and `max_value` to your own values. Values between your actual min/max will exclude datapoints, while values outside your actual min/max will compress your data into fewer sparks.

milc/ansi.py

@@ -4,6 +4,7 @@
 import re
 import logging
 import colorama
+from typing import Any
 
 from .emoji import EMOJI_LOGLEVELS
 
@@ -41,7 +42,7 @@
         ansi_colors[prefix + '_' + color.lower()] = getattr(obj, color)
 
 
-def format_ansi(text):
+def format_ansi(text: str) -> str:
     """Return a copy of text with certain strings replaced with ansi.
     """
     # Avoid .format() so we don't have to worry about the log content
@@ -59,9 +60,10 @@ def format_ansi(text):
 class MILCFormatter(logging.Formatter):
     """Formats log records per the MILC configuration.
     """
-    def format(self, record):
+    def format(self, record: Any) -> Any:
         if ansi_config['unicode'] and record.levelname in EMOJI_LOGLEVELS:
             record.levelname = format_ansi(EMOJI_LOGLEVELS[record.levelname])
 
         msg = super().format(record)
+
         return format_ansi(msg)

milc/attrdict.py

@@ -1,43 +1,46 @@
+from typing import Any, Dict, List
+
+
 class AttrDict(object):
     """A dictionary that can also be accessed by attribute.
     """
-    def __contains__(self, key):
+    def __contains__(self, key: Any) -> bool:
         return self._data.__contains__(key)
 
-    def __iter__(self):
+    def __iter__(self) -> Any:
         return self._data.__iter__()
 
-    def __len__(self):
+    def __len__(self) -> Any:
         return self._data.__len__()
 
-    def __repr__(self):
+    def __repr__(self) -> Any:
         return self._data.__repr__()
 
-    def keys(self):
+    def keys(self) -> Any:
         return self._data.keys()
 
-    def items(self):
+    def items(self) -> Any:
         return self._data.items()
 
-    def values(self):
+    def values(self) -> Any:
         return self._data.values()
 
-    def __init__(self, *args, **kwargs):
-        self._data = {}
+    def __init__(self, *args: List[Any], **kwargs: Dict[Any, Any]) -> None:
+        self._data: Dict[Any, Any] = {}
 
-    def __getattr__(self, key):
+    def __getattr__(self, key: Any) -> Any:
         return self.__getitem__(key)
 
-    def __getitem__(self, key):
+    def __getitem__(self, key: Any) -> Any:
         """Returns an item.
         """
         return self._data[key]
 
-    def __setitem__(self, key, value):
+    def __setitem__(self, key: Any, value: Any) -> None:
         self._data[key] = value
         self.__setattr__(key, value)
 
-    def __delitem__(self, key):
+    def __delitem__(self, key: Any) -> None:
         if key in self._data:
             del self._data[key]
 
@@ -48,7 +51,7 @@ class SparseAttrDict(AttrDict):
     This class never raises IndexError, instead it will return None if a
     key does not yet exist.
     """
-    def __getitem__(self, key):
+    def __getitem__(self, key: Any) -> Any:
         """Returns an item, creating it if it doesn't already exist
         """
         if key not in self._data:

milc/configuration.py

@@ -1,3 +1,5 @@
+from typing import Any, Hashable, List
+
 from .attrdict import AttrDict
 
 
@@ -7,7 +9,7 @@ class Configuration(AttrDict):
     This class never raises IndexError, instead it will return None if a
     section or option does not yet exist.
     """
-    def __getitem__(self, key):
+    def __getitem__(self, key: Hashable) -> Any:
         """Returns a config section, creating it if it doesn't exist yet.
         """
         if key not in self._data:
@@ -17,11 +19,11 @@ def __getitem__(self, key):
 
 
 class ConfigurationSection(Configuration):
-    def __init__(self, parent, *args, **kwargs):
+    def __init__(self, parent: AttrDict, *args: Any, **kwargs: Any) -> None:
         super(ConfigurationSection, self).__init__(*args, **kwargs)
         self._parent = parent
 
-    def __getitem__(self, key):
+    def __getitem__(self, key: Hashable) -> Any:
         """Returns a config value, pulling from the `user` section as a fallback.
         This is called when the attribute is accessed either via the get method or through [ ] index.
         """
@@ -33,7 +35,7 @@ def __getitem__(self, key):
 
         return None
 
-    def __getattr__(self, key):
+    def __getattr__(self, key: str) -> Any:
         """Returns the config value from the `user` section.
         This is called when the attribute is accessed via dot notation but does not exist.
         """
@@ -42,7 +44,7 @@ def __getattr__(self, key):
 
         return None
 
-    def __setattr__(self, key, value):
+    def __setattr__(self, key: str, value: Any) -> None:
         """Sets dictionary value when an attribute is set.
         """
         super().__setattr__(key, value)
@@ -54,7 +56,9 @@ def __setattr__(self, key, value):
 class SubparserWrapper(object):
     """Wrap subparsers so we can track what options the user passed.
     """
-    def __init__(self, cli, submodule, subparser):
+
+    # We type `cli` as Any instead of MILC to avoid a circular import
+    def __init__(self, cli: Any, submodule: Any, subparser: Any) -> None:
         self.cli = cli
         self.submodule = submodule
         self.subparser = subparser
@@ -63,66 +67,70 @@ def __init__(self, cli, submodule, subparser):
             if not hasattr(self, attr):
                 setattr(self, attr, getattr(subparser, attr))
 
-    def completer(self, completer):
+    def completer(self, completer: Any) -> None:
         """Add an arpcomplete completer to this subcommand.
         """
         self.subparser.completer = completer
 
-    def add_argument(self, *args, **kwargs):
+    def add_argument(self, *args: Any, **kwargs: Any) -> None:
         """Add an argument for this subcommand.
 
         This also stores the default for the argument in `self.cli.default_arguments`.
         """
         if kwargs.get('action') == 'store_boolean':
             # Store boolean will call us again with the enable/disable flag arguments
-            return handle_store_boolean(self, *args, **kwargs)
+            handle_store_boolean(self, *args, **kwargs)
 
-        completer = None
+        else:
+            completer = None
 
-        if kwargs.get('completer'):
-            completer = kwargs['completer']
-            del kwargs['completer']
+            if kwargs.get('completer'):
+                completer = kwargs['completer']
+                del kwargs['completer']
 
-        self.cli.acquire_lock()
-        argument_name = get_argument_name(self.cli._arg_parser, *args, **kwargs)
+            self.cli.acquire_lock()
+            argument_name = get_argument_name(self.cli._arg_parser, *args, **kwargs)
 
-        if completer:
-            self.subparser.add_argument(*args, **kwargs).completer = completer
-        else:
-            self.subparser.add_argument(*args, **kwargs)
+            if completer:
+                self.subparser.add_argument(*args, **kwargs).completer = completer
+            else:
+                self.subparser.add_argument(*args, **kwargs)
 
-        if kwargs.get('action') == 'store_false':
-            self.cli._config_store_false.append(argument_name)
+            if kwargs.get('action') == 'store_false':
+                self.cli._config_store_false.append(argument_name)
 
-        if kwargs.get('action') == 'store_true':
-            self.cli._config_store_true.append(argument_name)
+            if kwargs.get('action') == 'store_true':
+                self.cli._config_store_true.append(argument_name)
 
-        if self.submodule not in self.cli.default_arguments:
-            self.cli.default_arguments[self.submodule] = {}
+            if self.submodule not in self.cli.default_arguments:
+                self.cli.default_arguments[self.submodule] = {}
 
-        self.cli.default_arguments[self.submodule][argument_name] = kwargs.get('default')
-        self.cli.release_lock()
+            self.cli.default_arguments[self.submodule][argument_name] = kwargs.get('default')
+            self.cli.release_lock()
 
 
-def get_argument_strings(arg_parser, *args, **kwargs):
+def get_argument_strings(arg_parser: Any, *args: Any, **kwargs: Any) -> List[str]:
     """Takes argparse arguments and returns a list of argument strings or positional names.
     """
     try:
-        return arg_parser._get_optional_kwargs(*args, **kwargs)['option_strings']
+        return arg_parser._get_optional_kwargs(*args, **kwargs)['option_strings']  # type: ignore[no-any-return]
+
     except ValueError:
         return [arg_parser._get_positional_kwargs(*args, **kwargs)['dest']]
 
 
-def get_argument_name(arg_parser, *args, **kwargs):
+def get_argument_name(arg_parser: Any, *args: Any, **kwargs: Any) -> Any:
     """Takes argparse arguments and returns the dest name.
     """
     try:
         return arg_parser._get_optional_kwargs(*args, **kwargs)['dest']
+
     except ValueError:
         return arg_parser._get_positional_kwargs(*args, **kwargs)['dest']
 
 
-def handle_store_boolean(self, *args, **kwargs):
+# FIXME: We should not be using self in this way
+def handle_store_boolean(self: Any, *args: Any, **kwargs: Any) -> Any:
     """Does the add_argument for action='store_boolean'.
     """
     disabled_args = None