Skip to content

Commit

Permalink
feat: Reimplement expr as a class that is understood by IDEs (#3466)
Browse files Browse the repository at this point in the history
* fix: Use absolute imports explicitly

Fixes the mypy issue referenced in `alt.__init__.py`

* refactor: remove dynamic globals manipulation

The results of `_populate_namespace` were only visible in a `IPython` environment

* feat: Reimplement `expr` as a class w/ identicial semantics as the singleton

- all `ConstExpression`s are now read-only properties stored in a metaclass
- `expr` subclasses `ExprRef`, replacing the `__call__` w/ `__new__`
  - No instances of `expr` are ever created
- All `FunctionExpression`s are now classmethods
- Each `Expression` can now be referenced in autocomplete, without needing `IPython`/`Jupyter`
- Docstrings have been reformatted to be compatible with `sphinx`
- Broken links in docstrings fixed (e.g. for d3)
- class-level doc for `expr`, with references & doctest

* fix: use absolute imports in `test_expr`

Previously relied on the dynamic globals

* test(perf): rewrite `expr` tests to run in parallel

Previously 2 tests, now 170+

* test: confirm `expr` constants stay constant

* chore(typing): add ignores not flagged by mypy

* docs: adds `alt.expr` to API Reference

Uses the section heading proposed in #3427 (comment)

Expands on #2904

* test: ensure `test_expr_consts_immutable` pattern works for all versions

* docs: fix typos, regex artifacts, apply some `ruff` `D` rules

* docs: add doc for `_ConstExpressionType`

* test: make `expr` doctest testable

I left this out initially, but by adding static `Parameter` names it is now possible

* fix: re-run `generate-schema-wrapper`

https://github.com/vega/altair/actions/runs/10005909777/job/27657526013?pr=3466

* refactor: Remove `expr` test dependency on constants

Fixes #3466 (comment)

* style: remove trailing commas in `@pytest.mark.parametrize`
  • Loading branch information
dangotbanned committed Jul 20, 2024
1 parent 80939e7 commit b9d3fd4
Show file tree
Hide file tree
Showing 10 changed files with 1,500 additions and 90 deletions.
1,437 changes: 1,424 additions & 13 deletions altair/expr/__init__.py

Large diffs are not rendered by default.

15 changes: 0 additions & 15 deletions altair/expr/consts.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
from __future__ import annotations

from .core import ConstExpression


CONST_LISTING = {
"NaN": "not a number (same as JavaScript literal NaN)",
Expand All @@ -14,16 +12,3 @@
"SQRT2": "the square root of 2 (alias to Math.SQRT1_2)",
"PI": "the transcendental number pi (alias to Math.PI)",
}

NAME_MAP: dict[str, str] = {}


def _populate_namespace():
globals_ = globals()
for name, doc in CONST_LISTING.items():
py_name = NAME_MAP.get(name, name)
globals_[py_name] = ConstExpression(name, doc)
yield py_name


__all__ = list(_populate_namespace())
5 changes: 2 additions & 3 deletions altair/expr/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,9 +210,8 @@ def __repr__(self):


class ConstExpression(Expression):
def __init__(self, name, doc) -> None:
self.__doc__ = f"""{name}: {doc}"""
super().__init__(name=name, doc=doc)
def __init__(self, name) -> None:
super().__init__(name=name)

def __repr__(self) -> str:
return str(self.name)
Expand Down
30 changes: 0 additions & 30 deletions altair/expr/funcs.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
from __future__ import annotations
from typing import TYPE_CHECKING
from .core import FunctionExpression

if TYPE_CHECKING:
from typing import Iterator


FUNCTION_LISTING = {
"isArray": r"Returns true if _value_ is an array, false otherwise.",
Expand Down Expand Up @@ -171,27 +165,3 @@

# This maps vega expression function names to the Python name
NAME_MAP = {"if": "if_"}


class ExprFunc:
def __init__(self, name, doc) -> None:
self.name = name
self.doc = doc
self.__doc__ = f"""{name}(*args)\n {doc}"""

def __call__(self, *args) -> FunctionExpression:
return FunctionExpression(self.name, args)

def __repr__(self) -> str:
return f"<function expr.{self.name}(*args)>"


def _populate_namespace() -> Iterator[str]:
globals_ = globals()
for name, doc in FUNCTION_LISTING.items():
py_name = NAME_MAP.get(name, name)
globals_[py_name] = ExprFunc(name, doc)
yield py_name


__all__ = list(_populate_namespace())
2 changes: 1 addition & 1 deletion altair/vegalite/v5/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from .schema import *
from .api import *

from ...expr import datum, expr
from altair.expr.core import datum

from .display import VegaLite, renderers
from .compiler import vegalite_compilers
Expand Down
1 change: 1 addition & 0 deletions doc/user_guide/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,7 @@ API Utility Classes
:toctree: generated/api-cls/
:nosignatures:

expr
When
Then
ChainedWhen
75 changes: 57 additions & 18 deletions tests/expr/test_expr.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,39 @@
from __future__ import annotations

import operator
import sys
from inspect import classify_class_attrs, getmembers
from typing import Any, Iterator

import pytest

from altair import expr
from altair import datum
from altair import ExprRef
from jsonschema.exceptions import ValidationError

from altair import ExprRef, datum, expr
from altair.expr import _ConstExpressionType

# This maps vega expression function names to the Python name
VEGA_REMAP = {"if_": "if"}


def _is_property(obj: Any, /) -> bool:
return isinstance(obj, property)


def _get_classmethod_names(tp: type[Any], /) -> Iterator[str]:
for m in classify_class_attrs(tp):
if m.kind == "class method" and m.defining_class is tp:
yield m.name


def _remap_classmethod_names(tp: type[Any], /) -> Iterator[tuple[str, str]]:
for name in _get_classmethod_names(tp):
yield VEGA_REMAP.get(name, name), name


def _get_property_names(tp: type[Any], /) -> Iterator[str]:
for nm, _ in getmembers(tp, _is_property):
yield nm


def test_unary_operations():
OP_MAP = {"-": operator.neg, "+": operator.pos}
Expand Down Expand Up @@ -59,22 +86,34 @@ def test_abs():
assert repr(z) == "abs(datum.xxx)"


def test_expr_funcs():
@pytest.mark.parametrize(("veganame", "methodname"), _remap_classmethod_names(expr))
def test_expr_funcs(veganame: str, methodname: str):
"""test all functions defined in expr.funcs"""
name_map = {val: key for key, val in expr.funcs.NAME_MAP.items()}
for funcname in expr.funcs.__all__:
func = getattr(expr, funcname)
z = func(datum.xxx)
assert repr(z) == f"{name_map.get(funcname, funcname)}(datum.xxx)"
func = getattr(expr, methodname)
z = func(datum.xxx)
assert repr(z) == f"{veganame}(datum.xxx)"


def test_expr_consts():
@pytest.mark.parametrize("constname", _get_property_names(_ConstExpressionType))
def test_expr_consts(constname: str):
"""Test all constants defined in expr.consts"""
name_map = {val: key for key, val in expr.consts.NAME_MAP.items()}
for constname in expr.consts.__all__:
const = getattr(expr, constname)
z = const * datum.xxx
assert repr(z) == f"({name_map.get(constname, constname)} * datum.xxx)"

const = getattr(expr, constname)
z = const * datum.xxx
assert repr(z) == f"({constname} * datum.xxx)"


@pytest.mark.parametrize("constname", _get_property_names(_ConstExpressionType))
def test_expr_consts_immutable(constname: str):
"""Ensure e.g `alt.expr.PI = 2` is prevented."""
if sys.version_info >= (3, 11):
pattern = f"property {constname!r}.+has no setter"
elif sys.version_info >= (3, 10):
pattern = f"can't set attribute {constname!r}"
else:
pattern = "can't set attribute"
with pytest.raises(AttributeError, match=pattern):
setattr(expr, constname, 2)


def test_json_reprs():
Expand Down Expand Up @@ -127,7 +166,7 @@ def test_expression_function_nostring():
# expr() can only work with str otherwise
# should raise a SchemaValidationError
with pytest.raises(ValidationError):
expr(2 * 2)
expr(2 * 2) # pyright: ignore

with pytest.raises(ValidationError):
expr(["foo", "bah"])
expr(["foo", "bah"]) # pyright: ignore
16 changes: 10 additions & 6 deletions tests/vegalite/v5/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -593,13 +593,15 @@ def test_when_expressions_inside_parameters() -> None:
)
cond = when_then_otherwise["condition"][0]
otherwise = when_then_otherwise["value"]
expected = alt.expr(alt.expr.if_(alt.datum.b >= 0, 10, -20))
actual = alt.expr(alt.expr.if_(cond["test"], cond["value"], otherwise))
expected = alt.expr.if_(alt.datum.b >= 0, 10, -20)
actual = alt.expr.if_(cond["test"], cond["value"], otherwise)
assert expected == actual

text_conditioned = bar.mark_text(align="left", baseline="middle", dx=actual).encode(
text="b"
)
text_conditioned = bar.mark_text(
align="left",
baseline="middle",
dx=alt.expr(actual), # type: ignore[arg-type]
).encode(text="b")

chart = bar + text_conditioned
chart.to_dict()
Expand Down Expand Up @@ -719,12 +721,14 @@ def test_selection_to_dict():


def test_selection_expression():
from altair.expr.core import Expression

selection = alt.selection_point(fields=["value"])

assert isinstance(selection.value, alt.SelectionExpression)
assert selection.value.to_dict() == {"expr": f"{selection.name}.value"}

assert isinstance(selection["value"], alt.expr.Expression)
assert isinstance(selection["value"], Expression)
assert selection["value"].to_dict() == f"{selection.name}['value']"

magic_attr = "__magic__"
Expand Down
4 changes: 3 additions & 1 deletion tests/vegalite/v5/test_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,15 @@ def test_parameter_naming():


def test_selection_expression():
from altair.expr.core import Expression

data = pd.DataFrame([{"a": "A", "b": 28}])

sel = alt.selection_point(fields=["b"])
se = sel.b | 300

assert isinstance(se, alt.SelectionExpression)
assert isinstance(se.expr, alt.expr.core.Expression)
assert isinstance(se.expr, Expression)

c = (
alt.Chart(data)
Expand Down
5 changes: 2 additions & 3 deletions tools/generate_api_docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,8 @@ def api_functions() -> list[str]:


def api_classes() -> list[str]:
# classes defined in `api` and returned by `API Functions`,
# but not covered in other groups
return ["When", "Then", "ChainedWhen"]
# Part of the Public API, but are not inherited from `vega-lite`.
return ["expr", "When", "Then", "ChainedWhen"]


def lowlevel_wrappers() -> list[str]:
Expand Down

0 comments on commit b9d3fd4

Please sign in to comment.