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

feat: Reimplement expr as a class that is understood by IDEs #3466

Merged
merged 23 commits into from
Jul 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
0813874
fix: Use absolute imports explicitly
dangotbanned Jul 9, 2024
bcb8f1d
refactor: remove dynamic globals manipulation
dangotbanned Jul 9, 2024
ec32938
feat: Reimplement `expr` as a class w/ identicial semantics as the si…
dangotbanned Jul 9, 2024
b634913
fix: use absolute imports in `test_expr`
dangotbanned Jul 9, 2024
02fccf5
test(perf): rewrite `expr` tests to run in parallel
dangotbanned Jul 9, 2024
77a2073
test: confirm `expr` constants stay constant
dangotbanned Jul 9, 2024
fe09616
chore(typing): add ignores not flagged by mypy
dangotbanned Jul 9, 2024
8a80357
docs: adds `alt.expr` to API Reference
dangotbanned Jul 9, 2024
2b7a2b3
test: ensure `test_expr_consts_immutable` pattern works for all versions
dangotbanned Jul 9, 2024
e1182d4
Merge branch 'main' into expr-non-dynamic
dangotbanned Jul 9, 2024
34430a0
docs: fix typos, regex artifacts, apply some `ruff` `D` rules
dangotbanned Jul 10, 2024
c360965
Merge branch 'main' into expr-non-dynamic
dangotbanned Jul 13, 2024
448f1cf
Merge branch 'main' into expr-non-dynamic
dangotbanned Jul 13, 2024
4b2e0c2
Merge branch 'main' into expr-non-dynamic
dangotbanned Jul 15, 2024
ed6a819
Merge branch 'main' into expr-non-dynamic
dangotbanned Jul 16, 2024
a33d075
docs: add doc for `_ConstExpressionType`
dangotbanned Jul 16, 2024
f4e7b65
test: make `expr` doctest testable
dangotbanned Jul 16, 2024
7c29281
Merge branch 'main' into expr-non-dynamic
dangotbanned Jul 18, 2024
7185238
Merge branch 'main' into expr-non-dynamic
dangotbanned Jul 19, 2024
08448d1
fix: re-run `generate-schema-wrapper`
dangotbanned Jul 19, 2024
65b527a
refactor: Remove `expr` test dependency on constants
dangotbanned Jul 19, 2024
f78c401
Merge remote-tracking branch 'upstream/main' into expr-non-dynamic
dangotbanned Jul 19, 2024
ece2738
style: remove trailing commas in `@pytest.mark.parametrize`
dangotbanned Jul 19, 2024
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
1,437 changes: 1,424 additions & 13 deletions altair/expr/__init__.py
dangotbanned marked this conversation as resolved.
Show resolved Hide resolved

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