Skip to content

Commit

Permalink
fix: Restrict static & runtime top-level imports (#3482)
Browse files Browse the repository at this point in the history
* feat: generate an `__all__` for `channels.py`

This missing is the main source of unpredictable imports.

I've explicitly not included the `Mixin` classes, side effect imports (e.g. `Undefined`, `parse_shorthand`, `@overload`) and `@with_property_setters`

* build: run `generate-schema-wrapper`

* refactor: change the sources for some import in `api`

* refactor: add explicit import for `utils.parse_shorthand`

This was the only side-effect import that I imagine was useful

* build: run `update-init-file`

* chore(ruff): replace blanket `noqa` with specific rules

* refactor: add explicit imports in `v5.__init__.py`

Previously imported in `api` -> here

* feat: define an `__all__` for `api.py` manually

Currently not included deprecations, but will probably have to revert later

* refactor: add explicit imports for `Optional`, `Undefined`

* build: run `update-init-file`

Dropped from top-level:
`DataType`, `TOPLEVEL_ONLY_KEYS`, `mixins`

* test: use explicit import for `Undefined`

* test: use explicit import for `AltairDeprecationWarning`

* fix: add deprecations to `api.py`s `__all__`

* revert: Include `mixins`, `DataType`, `TOPLEVEL_ONLY_KEYS` in `api.__all__`

#3482 (comment)

* chore: remove TODO

* revert: include channels mixins and `with_property_setters` in `channels.__all__`

* build: run `generate-schema-wrapper`

* build: run `update-init-file`
  • Loading branch information
dangotbanned committed Jul 22, 2024
1 parent 3f96b74 commit 2b2f0b8
Show file tree
Hide file tree
Showing 9 changed files with 237 additions and 44 deletions.
2 changes: 1 addition & 1 deletion altair/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,7 @@ def __dir__():
from altair.vegalite.v5.schema.core import Dict
from altair.jupyter import JupyterChart
from altair.expr import expr
from altair.utils import AltairDeprecationWarning
from altair.utils import AltairDeprecationWarning, parse_shorthand, Optional, Undefined


def load_ipython_extension(ipython):
Expand Down
2 changes: 1 addition & 1 deletion altair/vegalite/__init__.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
# ruff: noqa
# ruff: noqa: F403
from .v5 import *
2 changes: 1 addition & 1 deletion altair/vegalite/schema.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""Altair schema wrappers"""

# ruff: noqa
# ruff: noqa: F403
from .v5.schema import *
11 changes: 9 additions & 2 deletions altair/vegalite/v5/__init__.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
# ruff: noqa
# ruff: noqa: F401, F403
from .schema import *
from .api import *

from altair.expr.core import datum

from .display import VegaLite, renderers
from .display import (
VegaLite,
renderers,
VEGALITE_VERSION,
VEGAEMBED_VERSION,
VEGA_VERSION,
)
from .compiler import vegalite_compilers

from .data import (
Expand All @@ -17,3 +23,4 @@
default_data_transformer,
data_transformers,
)
from .theme import themes
57 changes: 55 additions & 2 deletions altair/vegalite/v5/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@
import functools
import operator
from copy import deepcopy as _deepcopy
from .schema import core, channels, mixins, Undefined, SCHEMA_URL

from altair.utils import Optional
from .schema import core, channels, mixins, SCHEMA_URL

from altair.utils import Optional, Undefined
from .data import data_transformers
from ... import utils
from ...expr import core as _expr_core
Expand Down Expand Up @@ -123,6 +124,58 @@
OneOrSeq,
)

__all__ = [
"TOPLEVEL_ONLY_KEYS",
"Bin",
"ChainedWhen",
"Chart",
"ChartDataType",
"ChartType",
"ConcatChart",
"DataType",
"FacetChart",
"FacetMapping",
"HConcatChart",
"Impute",
"LayerChart",
"LookupData",
"Parameter",
"ParameterExpression",
"RepeatChart",
"SelectionExpression",
"SelectionPredicateComposition",
"Then",
"Title",
"TopLevelMixin",
"VConcatChart",
"When",
"binding",
"binding_checkbox",
"binding_radio",
"binding_range",
"binding_select",
"check_fields_and_encodings",
"concat",
"condition",
"graticule",
"hconcat",
"is_chart_type",
"layer",
"mixins",
"param",
"repeat",
"selection",
"selection_interval",
"selection_multi",
"selection_point",
"selection_single",
"sequence",
"sphere",
"topo_feature",
"value",
"vconcat",
"when",
]

ChartDataType: TypeAlias = Optional[Union[DataType, core.Data, str, core.Generator]]
_TSchemaBase = TypeVar("_TSchemaBase", bound=core.SchemaBase)
Expand Down
107 changes: 107 additions & 0 deletions altair/vegalite/v5/schema/channels.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,113 @@
from ._typing import * # noqa: F403


__all__ = [
"X2",
"Y2",
"Angle",
"AngleDatum",
"AngleValue",
"Color",
"ColorDatum",
"ColorValue",
"Column",
"DatumChannelMixin",
"Description",
"DescriptionValue",
"Detail",
"Facet",
"FieldChannelMixin",
"Fill",
"FillDatum",
"FillOpacity",
"FillOpacityDatum",
"FillOpacityValue",
"FillValue",
"Href",
"HrefValue",
"Key",
"Latitude",
"Latitude2",
"Latitude2Datum",
"Latitude2Value",
"LatitudeDatum",
"Longitude",
"Longitude2",
"Longitude2Datum",
"Longitude2Value",
"LongitudeDatum",
"Opacity",
"OpacityDatum",
"OpacityValue",
"Order",
"OrderValue",
"Radius",
"Radius2",
"Radius2Datum",
"Radius2Value",
"RadiusDatum",
"RadiusValue",
"Row",
"Shape",
"ShapeDatum",
"ShapeValue",
"Size",
"SizeDatum",
"SizeValue",
"Stroke",
"StrokeDash",
"StrokeDashDatum",
"StrokeDashValue",
"StrokeDatum",
"StrokeOpacity",
"StrokeOpacityDatum",
"StrokeOpacityValue",
"StrokeValue",
"StrokeWidth",
"StrokeWidthDatum",
"StrokeWidthValue",
"Text",
"TextDatum",
"TextValue",
"Theta",
"Theta2",
"Theta2Datum",
"Theta2Value",
"ThetaDatum",
"ThetaValue",
"Tooltip",
"TooltipValue",
"Url",
"UrlValue",
"ValueChannelMixin",
"X",
"X2Datum",
"X2Value",
"XDatum",
"XError",
"XError2",
"XError2Value",
"XErrorValue",
"XOffset",
"XOffsetDatum",
"XOffsetValue",
"XValue",
"Y",
"Y2Datum",
"Y2Value",
"YDatum",
"YError",
"YError2",
"YError2Value",
"YErrorValue",
"YOffset",
"YOffsetDatum",
"YOffsetValue",
"YValue",
"with_property_setters",
]


class FieldChannelMixin:
_encoding_name: str

Expand Down
19 changes: 10 additions & 9 deletions tests/vegalite/v5/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import polars as pl

import altair as alt
from altair.utils.schemapi import Undefined

try:
import vl_convert as vlc
Expand Down Expand Up @@ -1351,14 +1352,14 @@ def test_subcharts_with_same_data(method, data):
text = point.mark_text()

chart1 = func(point, line, text)
assert chart1.data is not alt.Undefined
assert all(c.data is alt.Undefined for c in getattr(chart1, method))
assert chart1.data is not Undefined
assert all(c.data is Undefined for c in getattr(chart1, method))

if method != "concat":
op = OP_DICT[method]
chart2 = op(op(point, line), text)
assert chart2.data is not alt.Undefined
assert all(c.data is alt.Undefined for c in getattr(chart2, method))
assert chart2.data is not Undefined
assert all(c.data is Undefined for c in getattr(chart2, method))


@pytest.mark.parametrize("method", ["layer", "hconcat", "vconcat", "concat"])
Expand All @@ -1373,20 +1374,20 @@ def test_subcharts_different_data(method, data):
nodata = alt.Chart().mark_point().encode(x="x:Q", y="y:Q")

chart1 = func(point, otherdata)
assert chart1.data is alt.Undefined
assert chart1.data is Undefined
assert getattr(chart1, method)[0].data is data

chart2 = func(point, nodata)
assert chart2.data is alt.Undefined
assert chart2.data is Undefined
assert getattr(chart2, method)[0].data is data


def test_layer_facet(basic_chart):
chart = (basic_chart + basic_chart).facet(row="row:Q")
assert chart.data is not alt.Undefined
assert chart.spec.data is alt.Undefined
assert chart.data is not Undefined
assert chart.spec.data is Undefined
for layer in chart.spec.layer:
assert layer.data is alt.Undefined
assert layer.data is Undefined

dct = chart.to_dict()
assert "data" in dct
Expand Down
7 changes: 4 additions & 3 deletions tests/vegalite/v5/test_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import re

import altair.vegalite.v5 as alt
from altair.utils.deprecation import AltairDeprecationWarning


def test_variable_param():
Expand Down Expand Up @@ -82,9 +83,9 @@ def test_selection_deprecation():
alt.selection_interval()

# this v4 syntax is deprecated
with pytest.warns(alt.utils.deprecation.AltairDeprecationWarning):
with pytest.warns(AltairDeprecationWarning):
alt.selection_single()
with pytest.warns(alt.utils.deprecation.AltairDeprecationWarning):
with pytest.warns(AltairDeprecationWarning):
alt.selection_multi()

# new syntax
Expand All @@ -95,7 +96,7 @@ def test_selection_deprecation():
# this v4 syntax is deprecated
brush = alt.selection_interval()
c = alt.Chart().mark_point()
with pytest.warns(alt.utils.deprecation.AltairDeprecationWarning):
with pytest.warns(AltairDeprecationWarning):
c.add_selection(brush)


Expand Down
Loading

0 comments on commit 2b2f0b8

Please sign in to comment.