Skip to content

Commit

Permalink
feat(typing): Fully annotate api.py (#3508)
Browse files Browse the repository at this point in the history
* ci(ruff): Add `ANN` rules for `api.py` only

To highlight all the missing annotations to fix and autofix `None` return

* feat(typing): Complete annotations for most `api` functions

Excluding `*args` on `ChartType` wrappers. They need to be defined in alignment in multiple places, which is more complex

* feat(typing): Annotate more expr/params in `api`

* feat(typing): Various changes to enforce `dict[str, Any]` instead of `dict`

Among these, many locations already assume `str` keys in the implementation - without checking

* feat(typing): Misc minor method annotations

* feat(typing): Use `ChartType` in all `ChartType` dunder methods

* fix(ruff): Ignore some `ANN` rules that won't be fixed

* chore: add pyright ignore from #3492

* feat(typing): Improve `ChartType` constructor/factory annotations

* feat(typing): Annotate remaining functions in `api`

* feat(typing): Complete `RepeatChart` annotations

* fix(typing): Resolve Liskov violations

```
altair\vegalite\v5\api.py:4368: error: Argument 1 of "__iadd__" is incompatible with "__add__" of supertype "TopLevelMixin"; supertype defines the argument type as "Chart | RepeatChart | ConcatChart | HConcatChart | VConcatChart | FacetChart | LayerChart"  [override]         def __iadd__(self, other: LayerChart | Chart) -> Self:                            ^~~~~~~~~~~~~~~~~~~~~~~~~ altair\vegalite\v5\api.py:4368: note: This violates the Liskov substitution principle altair\vegalite\v5\api.py:4368: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides altair\vegalite\v5\api.py:4376: error: Argument 1 of "__add__" is incompatible with supertype "TopLevelMixin"; supertype defines the argument type as "Chart | RepeatChart | ConcatChart | HConcatChart | VConcatChart | FacetChart | LayerChart"  [override]         def __add__(self, other: LayerChart | Chart) -> Self:                           ^~~~~~~~~~~~~~~~~~~~~~~~~ altair\vegalite\v5\api.py:4376: note: This violates the Liskov substitution principle altair\vegalite\v5\api.py:4376: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
```

* chore(typing): Update more `dict` -> `dict[str, Any]`

* style(ruff): fix whitespace

* chore: Remove TODO fixed in #3480

* fix(typing): Enable `ANN003` and fix all in `api`

* fix(typing): Enable `ANN002` and fix all in `api`

* chore(ruff): Add note on `ANN`

This could later be extended to other modules, but for now `api` is complete.

* fix(typing): Add missing `FacetChart` annotations

To align with the other `ChartType`s
  • Loading branch information
dangotbanned authored Aug 2, 2024
1 parent d2325e3 commit a847833
Show file tree
Hide file tree
Showing 6 changed files with 300 additions and 185 deletions.
9 changes: 8 additions & 1 deletion altair/expr/core.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
from __future__ import annotations
from typing import Any

from typing import Any, Union, Dict

from typing_extensions import TypeAlias

from ..utils import SchemaBase


Expand Down Expand Up @@ -232,3 +236,6 @@ def __init__(self, group, name) -> None:

def __repr__(self) -> str:
return f"{self.group}[{self.name!r}]"


IntoExpression: TypeAlias = Union[bool, None, str, OperatorMixin, Dict[str, Any]]
6 changes: 3 additions & 3 deletions altair/utils/_transformed_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

if TYPE_CHECKING:
from altair.utils.core import DataFrameLike
from altair.vegalite.v5.api import ChartType

Scope: TypeAlias = Tuple[int, ...]
FacetMapping: TypeAlias = Dict[Tuple[str, Scope], Tuple[str, Scope]]
Expand Down Expand Up @@ -154,9 +155,7 @@ def transformed_data(chart, row_limit=None, exclude=None):
# The same error appeared when trying it with Protocols for the concat and layer charts.
# This function is only used internally and so we accept this inconsistency for now.
def name_views(
chart: Chart | FacetChart | LayerChart | HConcatChart | VConcatChart | ConcatChart,
i: int = 0,
exclude: Iterable[str] | None = None,
chart: ChartType, i: int = 0, exclude: Iterable[str] | None = None
) -> list[str]:
"""
Name unnamed chart views.
Expand Down Expand Up @@ -193,6 +192,7 @@ def name_views(
else:
return []
else:
subcharts: list[Any]
if isinstance(chart, _chart_class_mapping[LayerChart]):
subcharts = chart.layer
elif isinstance(chart, _chart_class_mapping[HConcatChart]):
Expand Down
2 changes: 1 addition & 1 deletion altair/utils/_vegafusion_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ def compile_to_vegafusion_chart_state(
return chart_state


def compile_with_vegafusion(vegalite_spec: dict[str, Any]) -> dict:
def compile_with_vegafusion(vegalite_spec: dict[str, Any]) -> dict[str, Any]:
"""
Compile a Vega-Lite spec to Vega and pre-transform with VegaFusion.
Expand Down
6 changes: 3 additions & 3 deletions altair/utils/compiler.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
from typing import Callable
from typing import Callable, Dict, Any
from altair.utils import PluginRegistry

# ==============================================================================
# Vega-Lite to Vega compiler registry
# ==============================================================================
VegaLiteCompilerType = Callable[[dict], dict]
VegaLiteCompilerType = Callable[[Dict[str, Any]], Dict[str, Any]]


class VegaLiteCompilerRegistry(PluginRegistry[VegaLiteCompilerType, dict]):
class VegaLiteCompilerRegistry(PluginRegistry[VegaLiteCompilerType, Dict[str, Any]]):
pass
Loading

0 comments on commit a847833

Please sign in to comment.