Type hints: Add static type checker mypy

.github/workflows/lint.yml

@@ -5,19 +5,25 @@ on: [push, pull_request]
 jobs:
   build:
     runs-on: ubuntu-latest
-    name: black-flake8
+    name: black-flake8-mypy
     steps:
     - uses: actions/checkout@v3
     - name: Set up Python 3.10
       uses: actions/setup-python@v4
       with:
         python-version: "3.10"
+    # Installing all dependencies and not just the linters as mypy needs them for type checking
+    - name: Install dependencies
+      run: |
+        python -m pip install --upgrade pip
+        pip install .[dev]
     - name: Check formatting with black
       run: |
-        pip install "black<24"
         black --diff --color .
         black --check .
     - name: Lint with flake8
       run: |
-        pip install flake8
         flake8 .  --statistics
+    - name: Lint with mypy
+      run: |
+        mypy altair tests

Makefile

@@ -6,6 +6,7 @@ install:
 test :
 	black --diff --color --check .
 	flake8 . --statistics
+	mypy altair tests
 	python -m pytest --pyargs --doctest-modules tests
 
 test-coverage:

altair/__init__.py

@@ -1,6 +1,12 @@
 # flake8: noqa
 __version__ = "5.0.0dev"
 
+from typing import Any
+
+# Necessary as mypy would see expr as the module alt.expr although due to how
+# the imports are set up it is expr in the alt.expr module
+expr: Any
+
 
 # The content of __all__ is automatically written by
 # tools/update_init_file.py. Do not modify directly.

altair/expr/__init__.py

@@ -1,5 +1,7 @@
 """Tools for creating transform & filter expressions with a python syntax"""
 # flake8: noqa
+from typing import Any
+
 from .core import datum, Expression
 from .funcs import *
 from .consts import *
@@ -14,4 +16,4 @@ def __call__(self, expr, **kwargs):
         return _ExprRef(expr, **kwargs)
 
 
-expr = _ExprType(globals())
+expr: Any = _ExprType(globals())

altair/expr/consts.py

@@ -1,3 +1,5 @@
+from typing import Dict
+
 from .core import ConstExpression
 
 
@@ -13,7 +15,7 @@
     "PI": "the transcendental number pi (alias to Math.PI)",
 }
 
-NAME_MAP = {}
+NAME_MAP: Dict[str, str] = {}
 
 
 def _populate_namespace():

altair/utils/core.py

@@ -26,7 +26,7 @@
     from pandas.api.types import infer_dtype as _infer_dtype
 except ImportError:
     # Import for pandas < 0.20.0
-    from pandas.lib import infer_dtype as _infer_dtype
+    from pandas.lib import infer_dtype as _infer_dtype  # type: ignore[no-redef]
 
 _V = TypeVar("_V")
 _P = ParamSpec("_P")
@@ -568,8 +568,8 @@ def use_signature(Obj: Callable[_P, Any]):
     def decorate(f: Callable[..., _V]) -> Callable[_P, _V]:
         # call-signature of f is exposed via __wrapped__.
         # we want it to mimic Obj.__init__
-        f.__wrapped__ = Obj.__init__
-        f._uses_signature = Obj
+        f.__wrapped__ = Obj.__init__  # type: ignore
+        f._uses_signature = Obj  # type: ignore
 
         # Supplement the docstring of f with information from Obj
         if Obj.__doc__:

altair/utils/display.py

@@ -1,7 +1,7 @@
 import json
 import pkgutil
 import textwrap
-from typing import Callable, Dict
+from typing import Callable, Dict, Optional
 import uuid
 
 from .plugin_registry import PluginRegistry
@@ -112,7 +112,7 @@ class Displayable:
     through appropriate data model transformers.
     """
 
-    renderers = None
+    renderers: Optional[RendererRegistry] = None
     schema_path = ("altair", "")
 
     def __init__(self, spec, validate=False):
@@ -124,7 +124,9 @@ def __init__(self, spec, validate=False):
     def _validate(self):
         # type: () -> None
         """Validate the spec against the schema."""
-        schema_dict = json.loads(pkgutil.get_data(*self.schema_path).decode("utf-8"))
+        data = pkgutil.get_data(*self.schema_path)
+        assert data is not None
+        schema_dict = json.loads(data.decode("utf-8"))
         validate_jsonschema(
             self.spec,
             schema_dict,

altair/utils/plugin_registry.py

@@ -1,9 +1,10 @@
+import sys
 from typing import Any, Dict, List, Optional, Generic, TypeVar, cast
 from types import TracebackType
 
-try:
+if sys.version_info >= (3, 8):
     from importlib.metadata import entry_points
-except ImportError:
+else:
     from importlib_metadata import entry_points
 
 from toolz import curry
@@ -114,7 +115,7 @@ def register(self, name: str, value: Optional[PluginType]) -> Optional[PluginTyp
         if value is None:
             return self._plugins.pop(name, None)
         else:
-            assert isinstance(value, self.plugin_type)
+            assert isinstance(value, self.plugin_type)  # type: ignore[arg-type]  # Should ideally be fixed by better annotating plugin_type
             self._plugins[name] = value
             return value
 

altair/utils/schemapi.py

@@ -5,7 +5,7 @@
 import inspect
 import json
 import textwrap
-from typing import Any, Sequence, List
+from typing import Any, Sequence, List, Dict, Optional
 from itertools import zip_longest
 
 import jsonschema
@@ -105,11 +105,11 @@ def _get_most_relevant_errors(
         parent = lowest_level.parent
         if parent is None:
             # In this case we are still at the top level and can return all errors
-            most_relevant_errors = errors
+            most_relevant_errors = list(errors)
         else:
             # Use all errors of the lowest level out of which
             # we can construct more informative error messages
-            most_relevant_errors = lowest_level.parent.context
+            most_relevant_errors = parent.context or []
             if lowest_level.validator == "enum":
                 # There might be other possible enums which are allowed, e.g. for
                 # the "timeUnit" property of the "Angle" encoding channel. These do not
@@ -339,8 +339,8 @@ class SchemaBase:
     the _rootschema class attribute) which is used for validation.
     """
 
-    _schema = None
-    _rootschema = None
+    _schema: Optional[Dict[str, Any]] = None
+    _rootschema: Optional[Dict[str, Any]] = None
     _class_is_valid_at_instantiation = True
 
     def __init__(self, *args, **kwds):

altair/vegalite/v5/__init__.py

@@ -4,7 +4,7 @@
 
 from ...datasets import list_datasets, load_dataset
 
-from ...expr import datum, expr
+from ...expr import datum, expr  # type: ignore[no-redef]
 
 from .display import VegaLite, renderers
 

altair/vegalite/v5/api.py

@@ -8,6 +8,10 @@
 from toolz.curried import pipe as _pipe
 import itertools
 import sys
+from typing import cast
+
+# Have to rename it here as else it overlaps with schema.core.Type
+from typing import Type as TypingType
 
 from .schema import core, channels, mixins, Undefined, SCHEMA_URL
 
@@ -812,7 +816,9 @@ def to_dict(self, *args, **kwargs) -> dict:
         context.setdefault("datasets", {})
         is_top_level = context.get("top_level", True)
 
-        copy = self.copy(deep=False)
+        # TopLevelMixin instance does not necessarily have copy defined but due to how
+        # Altair is set up this should hold. Too complex to type hint right now
+        copy = self.copy(deep=False)  # type: ignore[attr-defined]
         original_data = getattr(copy, "data", Undefined)
         copy.data = _prepare_data(original_data, context)
 
@@ -823,7 +829,10 @@ def to_dict(self, *args, **kwargs) -> dict:
         context["top_level"] = False
         kwargs["context"] = context
 
-        dct = super(TopLevelMixin, copy).to_dict(*args, **kwargs)
+        # TopLevelMixin instance does not necessarily have to_dict defined
+        # but due to how Altair is set up this should hold.
+        # Too complex to type hint right now
+        dct = super(TopLevelMixin, copy).to_dict(*args, **kwargs)  # type: ignore[misc]
 
         # TODO: following entries are added after validation. Should they be validated?
         if is_top_level:
@@ -833,6 +842,9 @@ def to_dict(self, *args, **kwargs) -> dict:
 
             # apply theme from theme registry
             the_theme = themes.get()
+            # Use assert to tell type checkers that it is not None. Holds true
+            # as there is always a default theme set when importing Altair
+            assert the_theme is not None
             dct = utils.update_nested(the_theme(), dct, copy=True)
 
             # update datasets
@@ -1006,7 +1018,8 @@ def properties(self, **kwargs) -> Self:
 
         Argument names and types are the same as class initialization.
         """
-        copy = self.copy(deep=False)
+        # ignore type as copy comes from another class for subclasses of TopLevelMixin
+        copy = self.copy(deep=False)  # type: ignore[attr-defined]
         for key, val in kwargs.items():
             if key == "selection" and isinstance(val, Parameter):
                 # TODO: Can this be removed
@@ -1015,7 +1028,9 @@ def properties(self, **kwargs) -> Self:
             else:
                 # Don't validate data, because it hasn't been processed.
                 if key != "data":
-                    self.validate_property(key, val)
+                    # ignore type as validate_property comes from SchemaBase,
+                    # not from TopLevelMixin
+                    self.validate_property(key, val)  # type: ignore[attr-defined]
                 setattr(copy, key, val)
         return copy
 
@@ -2283,7 +2298,8 @@ def encode(self, *args, **kwargs) -> Self:
         kwargs = utils.infer_encoding_types(args, kwargs, channels)
 
         # get a copy of the dict representation of the previous encoding
-        copy = self.copy(deep=["encoding"])
+        # ignore type as copy method comes from SchemaBase
+        copy = self.copy(deep=["encoding"])  # type: ignore[attr-defined]
         encoding = copy._get("encoding", {})
         if isinstance(encoding, core.VegaLiteSchema):
             encoding = {k: v for k, v in encoding._kwds.items() if v is not Undefined}
@@ -2338,13 +2354,17 @@ def facet(
                 "facet argument cannot be combined with row/column argument."
             )
 
-        if data is Undefined:
-            if self.data is Undefined:
+        # Remove "ignore" statement once Undefined is no longer typed as Any
+        if data is Undefined:  # type: ignore
+            # Remove "ignore" statement once Undefined is no longer typed as Any
+            if self.data is Undefined:  # type: ignore
                 raise ValueError(
                     "Facet charts require data to be specified at the top level."
                 )
-            self = self.copy(deep=False)
-            data, self.data = self.data, Undefined
+            # ignore type as copy comes from another class
+            self = self.copy(deep=False)  # type: ignore[attr-defined]
+            # Remove "ignore" statement once Undefined is no longer typed as Any
+            data, self.data = self.data, Undefined  # type: ignore
 
         if facet_specified:
             if isinstance(facet, str):
@@ -2439,7 +2459,7 @@ def _get_name(cls):
         return f"view_{cls._counter}"
 
     @classmethod
-    def from_dict(cls, dct, validate=True) -> "Chart":
+    def from_dict(cls, dct, validate=True) -> "Chart":  # type: ignore[override]  # Not the same signature as SchemaBase.from_dict. Would ideally be aligned in the future
         """Construct class from a dictionary representation
 
         Parameters
@@ -2461,9 +2481,12 @@ def from_dict(cls, dct, validate=True) -> "Chart":
         """
         for class_ in TopLevelMixin.__subclasses__():
             if class_ is Chart:
-                class_ = super(Chart, cls)
+                class_ = cast(TypingType[TopLevelMixin], super(Chart, cls))
             try:
-                return class_.from_dict(dct, validate=validate)
+                # TopLevelMixin classes don't necessarily have from_dict defined
+                # but all classes which are used here have due to how Altair is
+                # designed. Too complex to type check right now.
+                return class_.from_dict(dct, validate=validate)  # type: ignore[attr-defined]
             except jsonschema.ValidationError:
                 pass