Skip to content

Commit

Permalink
Var: __bool__ and __iter__ always raise a TypeError (#1750)
Browse files Browse the repository at this point in the history
  • Loading branch information
masenf authored Sep 5, 2023
1 parent e99d672 commit 71811a6
Show file tree
Hide file tree
Showing 14 changed files with 128 additions and 75 deletions.
4 changes: 2 additions & 2 deletions reflex/.templates/jinja/web/pages/utils.js.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
{% filter indent(width=indent_width) %}
{%- if component is not mapping %}
{{- component }}
{%- elif component.iterable %}
{%- elif "iterable" in component %}
{{- render_iterable_tag(component) }}
{%- elif component.cond %}
{%- elif "cond" in component %}
{{- render_condition_tag(component) }}
{%- elif component.children|length %}
{{- render_tag(component) }}
Expand Down
10 changes: 7 additions & 3 deletions reflex/components/datadisplay/datatable.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,11 @@ def create(cls, *children, **props):
"Annotation of the computed var assigned to the data field should be provided."
)

if columns and isinstance(columns, ComputedVar) and columns.type_ == Any:
if (
columns is not None
and isinstance(columns, ComputedVar)
and columns.type_ == Any
):
raise ValueError(
"Annotation of the computed var assigned to the column field should be provided."
)
Expand All @@ -75,7 +79,7 @@ def create(cls, *children, **props):
if (
types.is_dataframe(type(data))
or (isinstance(data, Var) and types.is_dataframe(data.type_))
) and props.get("columns"):
) and columns is not None:
raise ValueError(
"Cannot pass in both a pandas dataframe and columns to the data_table component."
)
Expand All @@ -84,7 +88,7 @@ def create(cls, *children, **props):
if (
(isinstance(data, Var) and types._issubclass(data.type_, List))
or issubclass(type(data), List)
) and not props.get("columns"):
) and columns is None:
raise ValueError(
"column field should be specified when the data field is a list type"
)
Expand Down
2 changes: 1 addition & 1 deletion reflex/components/navigation/link.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def create(cls, *children, **props) -> Component:
Returns:
Component: The link component
"""
if props.get("href"):
if props.get("href") is not None:
if not len(children):
raise ValueError("Link without a child will not display")
else:
Expand Down
2 changes: 1 addition & 1 deletion reflex/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -885,7 +885,7 @@ def _mark_dirty_computed_vars(self) -> None:
self.dirty_vars.add(cvar)
dirty_vars.add(cvar)
actual_var = self.computed_vars.get(cvar)
if actual_var:
if actual_var is not None:
actual_var.mark_dirty(instance=self)

def _dirty_computed_vars(self, from_vars: set[str] | None = None) -> set[str]:
Expand Down
7 changes: 3 additions & 4 deletions reflex/utils/format.py
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ def format_ref(ref: str) -> str:
return f"ref_{clean_ref}"


def format_array_ref(refs: str, idx) -> str:
def format_array_ref(refs: str, idx: Var | None) -> str:
"""Format a ref accessed by array.
Args:
Expand All @@ -553,11 +553,10 @@ def format_array_ref(refs: str, idx) -> str:
The formatted ref.
"""
clean_ref = re.sub(r"[^\w]+", "_", refs)
if idx:
if idx is not None:
idx.is_local = True
return f"refs_{clean_ref}[{idx}]"
else:
return f"refs_{clean_ref}"
return f"refs_{clean_ref}"


def format_dict(prop: ComponentStyle) -> str:
Expand Down
23 changes: 22 additions & 1 deletion reflex/vars.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,27 @@ def __str__(self) -> str:
out = format.format_string(out)
return out

def __bool__(self) -> bool:
"""Raise exception if using Var in a boolean context.
Raises:
TypeError: when attempting to bool-ify the Var.
"""
raise TypeError(
f"Cannot convert Var {self.full_name!r} to bool for use with `if`, `and`, `or`, and `not`. "
"Instead use `rx.cond` and bitwise operators `&` (and), `|` (or), `~` (invert)."
)

def __iter__(self) -> Any:
"""Raise exception if using Var in an iterable context.
Raises:
TypeError: when attempting to iterate over the Var.
"""
raise TypeError(
f"Cannot iterate over Var {self.full_name!r}. Instead use `rx.foreach`."
)

def __format__(self, format_spec: str) -> str:
"""Format the var into a Javascript equivalent to an f-string.
Expand Down Expand Up @@ -1414,7 +1435,7 @@ def get_local_storage(key: Var | str | None = None) -> BaseVar:
Raises:
TypeError: if the wrong key type is provided.
"""
if key:
if key is not None:
if not (isinstance(key, Var) and key.type_ == str) and not isinstance(key, str):
type_ = type(key) if not isinstance(key, Var) else key.type_
raise TypeError(
Expand Down
9 changes: 5 additions & 4 deletions tests/components/forms/test_debounce.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ def test_render_child_props():
)
)._render()
assert tag.props["sx"] == {"foo": "bar", "baz": "quuc"}
assert tag.props["value"] == BaseVar(
name="real", type_=str, is_local=True, is_string=False
assert tag.props["value"].equals(
BaseVar(name="real", type_=str, is_local=True, is_string=False)
)
assert len(tag.props["onChange"].events) == 1
assert tag.props["onChange"].events[0].handler == S.on_change
Expand All @@ -75,6 +75,7 @@ def test_render_child_props_recursive():
on_change=S.on_change,
),
value="inner",
debounce_timeout=666,
force_notify_on_blur=False,
),
debounce_timeout=42,
Expand All @@ -84,8 +85,8 @@ def test_render_child_props_recursive():
force_notify_by_enter=False,
)._render()
assert tag.props["sx"] == {"foo": "bar", "baz": "quuc"}
assert tag.props["value"] == BaseVar(
name="real", type_=str, is_local=True, is_string=False
assert tag.props["value"].equals(
BaseVar(name="outer", type_=str, is_local=True, is_string=False)
)
assert tag.props["forceNotifyOnBlur"].name == "false"
assert tag.props["forceNotifyByEnter"].name == "false"
Expand Down
2 changes: 1 addition & 1 deletion tests/components/layout/test_cond.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def test_cond_no_else():
assert isinstance(comp, Fragment)
comp = comp.children[0]
assert isinstance(comp, Cond)
assert comp.cond == True # noqa
assert comp.cond._decode() is True # type: ignore
assert comp.comp1 == Fragment.create(Text.create("hello"))
assert comp.comp2 == Fragment.create()

Expand Down
2 changes: 1 addition & 1 deletion tests/components/layout/test_foreach.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,6 @@ def test_foreach_render(state_var, render_fn, render_dict):
rend = component.render()
arg_index = rend["arg_index"]
assert rend["iterable_state"] == render_dict["iterable_state"]
assert rend["arg_index"] == render_dict["arg_index"]
assert arg_index.name == render_dict["arg_index"]
assert arg_index.type_ == int
assert rend["iterable_type"] == render_dict["iterable_type"]
6 changes: 3 additions & 3 deletions tests/components/test_component.py
Original file line number Diff line number Diff line change
Expand Up @@ -329,8 +329,8 @@ def test_valid_props(component1, text: str, number: int):
number: A test number.
"""
c = component1.create(text=text, number=number)
assert c.text == text
assert c.number == number
assert c.text._decode() == text
assert c.number._decode() == number


@pytest.mark.parametrize(
Expand All @@ -357,7 +357,7 @@ def test_var_props(component1, test_state):
test_state: A test state.
"""
c1 = component1.create(text="hello", number=test_state.num)
assert c1.number == test_state.num
assert c1.number.equals(test_state.num)


def test_get_controlled_triggers(component1, component2):
Expand Down
10 changes: 6 additions & 4 deletions tests/components/test_tag.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ def test_is_valid_prop(prop: Var, valid: bool):
def test_add_props():
"""Test that the props are added."""
tag = Tag().add_props(key="value", key2=42, invalid=None, invalid2={})
assert tag.props["key"] == Var.create("value")
assert tag.props["key2"] == Var.create(42)
assert tag.props["key"].equals(Var.create("value"))
assert tag.props["key2"].equals(Var.create(42))
assert "invalid" not in tag.props
assert "invalid2" not in tag.props

Expand Down Expand Up @@ -100,7 +100,8 @@ def test_format_tag(tag: Tag, expected: Dict):
tag_dict = dict(tag)
assert tag_dict["name"] == expected["name"]
assert tag_dict["contents"] == expected["contents"]
assert tag_dict["props"] == expected["props"]
for prop, prop_value in tag_dict["props"].items():
assert prop_value.equals(Var.create_safe(expected["props"][prop]))


def test_format_cond_tag():
Expand All @@ -116,7 +117,8 @@ def test_format_cond_tag():
tag_dict["true_value"],
tag_dict["false_value"],
)
assert cond == "logged_in"
assert cond.name == "logged_in"
assert cond.type_ == bool

assert true_value["name"] == "h1"
assert true_value["contents"] == "True content"
Expand Down
3 changes: 2 additions & 1 deletion tests/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -930,5 +930,6 @@ async def test_process_events(gen_state, mocker):

async for _update in process(app, event, "mock_sid", {}, "127.0.0.1"):
pass
assert gen_state.value == 5

assert app.state_manager.get_state("token").value == 5
assert app.postprocess.call_count == 6
62 changes: 38 additions & 24 deletions tests/test_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ def test_fn_with_args(_, arg1, arg2):

# Test passing vars as args.
assert event_spec.handler == handler
assert event_spec.args == (("arg1", "first"), ("arg2", "second"))
assert event_spec.args[0][0].equals(Var.create_safe("arg1"))
assert event_spec.args[0][1].equals(Var.create_safe("first"))
assert event_spec.args[1][0].equals(Var.create_safe("arg2"))
assert event_spec.args[1][1].equals(Var.create_safe("second"))
assert (
format.format_event(event_spec)
== 'E("test_fn_with_args", {arg1:first,arg2:second})'
Expand All @@ -77,10 +80,10 @@ def test_fn_with_args(_, arg1, arg2):
)

assert event_spec.handler == handler
assert event_spec.args == (
("arg1", format.json_dumps(first)),
("arg2", format.json_dumps(second)),
)
assert event_spec.args[0][0].equals(Var.create_safe("arg1"))
assert event_spec.args[0][1].equals(Var.create_safe(first))
assert event_spec.args[1][0].equals(Var.create_safe("arg2"))
assert event_spec.args[1][1].equals(Var.create_safe(second))

handler = EventHandler(fn=test_fn_with_args)
with pytest.raises(TypeError):
Expand Down Expand Up @@ -121,7 +124,8 @@ def test_event_redirect():
spec = event.redirect("/path")
assert isinstance(spec, EventSpec)
assert spec.handler.fn.__qualname__ == "_redirect"
assert spec.args == (("path", "/path"),)
assert spec.args[0][0].equals(Var.create_safe("path"))
assert spec.args[0][1].equals(Var.create_safe("/path"))
assert format.format_event(spec) == 'E("_redirect", {path:"/path"})'
spec = event.redirect(Var.create_safe("path"))
assert format.format_event(spec) == 'E("_redirect", {path:path})'
Expand All @@ -132,7 +136,8 @@ def test_event_console_log():
spec = event.console_log("message")
assert isinstance(spec, EventSpec)
assert spec.handler.fn.__qualname__ == "_console"
assert spec.args == (("message", "message"),)
assert spec.args[0][0].equals(Var.create_safe("message"))
assert spec.args[0][1].equals(Var.create_safe("message"))
assert format.format_event(spec) == 'E("_console", {message:"message"})'
spec = event.console_log(Var.create_safe("message"))
assert format.format_event(spec) == 'E("_console", {message:message})'
Expand All @@ -143,7 +148,8 @@ def test_event_window_alert():
spec = event.window_alert("message")
assert isinstance(spec, EventSpec)
assert spec.handler.fn.__qualname__ == "_alert"
assert spec.args == (("message", "message"),)
assert spec.args[0][0].equals(Var.create_safe("message"))
assert spec.args[0][1].equals(Var.create_safe("message"))
assert format.format_event(spec) == 'E("_alert", {message:"message"})'
spec = event.window_alert(Var.create_safe("message"))
assert format.format_event(spec) == 'E("_alert", {message:message})'
Expand All @@ -154,7 +160,8 @@ def test_set_focus():
spec = event.set_focus("input1")
assert isinstance(spec, EventSpec)
assert spec.handler.fn.__qualname__ == "_set_focus"
assert spec.args == (("ref", Var.create_safe("ref_input1")),)
assert spec.args[0][0].equals(Var.create_safe("ref"))
assert spec.args[0][1].equals(Var.create_safe("ref_input1"))
assert format.format_event(spec) == 'E("_set_focus", {ref:ref_input1})'
spec = event.set_focus("input1")
assert format.format_event(spec) == 'E("_set_focus", {ref:ref_input1})'
Expand All @@ -165,10 +172,10 @@ def test_set_value():
spec = event.set_value("input1", "")
assert isinstance(spec, EventSpec)
assert spec.handler.fn.__qualname__ == "_set_value"
assert spec.args == (
("ref", Var.create_safe("ref_input1")),
("value", ""),
)
assert spec.args[0][0].equals(Var.create_safe("ref"))
assert spec.args[0][1].equals(Var.create_safe("ref_input1"))
assert spec.args[1][0].equals(Var.create_safe("value"))
assert spec.args[1][1].equals(Var.create_safe(""))
assert format.format_event(spec) == 'E("_set_value", {ref:ref_input1,value:""})'
spec = event.set_value("input1", Var.create_safe("message"))
assert (
Expand All @@ -181,10 +188,10 @@ def test_set_cookie():
spec = event.set_cookie("testkey", "testvalue")
assert isinstance(spec, EventSpec)
assert spec.handler.fn.__qualname__ == "_set_cookie"
assert spec.args == (
("key", "testkey"),
("value", "testvalue"),
)
assert spec.args[0][0].equals(Var.create_safe("key"))
assert spec.args[0][1].equals(Var.create_safe("testkey"))
assert spec.args[1][0].equals(Var.create_safe("value"))
assert spec.args[1][1].equals(Var.create_safe("testvalue"))
assert (
format.format_event(spec)
== 'E("_set_cookie", {key:"testkey",value:"testvalue"})'
Expand All @@ -196,7 +203,10 @@ def test_remove_cookie():
spec = event.remove_cookie("testkey")
assert isinstance(spec, EventSpec)
assert spec.handler.fn.__qualname__ == "_remove_cookie"
assert spec.args == (("key", "testkey"), ("options", {}))
assert spec.args[0][0].equals(Var.create_safe("key"))
assert spec.args[0][1].equals(Var.create_safe("testkey"))
assert spec.args[1][0].equals(Var.create_safe("options"))
assert spec.args[1][1].equals(Var.create_safe({}))
assert (
format.format_event(spec) == 'E("_remove_cookie", {key:"testkey",options:{}})'
)
Expand All @@ -213,7 +223,10 @@ def test_remove_cookie_with_options():
spec = event.remove_cookie("testkey", options)
assert isinstance(spec, EventSpec)
assert spec.handler.fn.__qualname__ == "_remove_cookie"
assert spec.args == (("key", "testkey"), ("options", options))
assert spec.args[0][0].equals(Var.create_safe("key"))
assert spec.args[0][1].equals(Var.create_safe("testkey"))
assert spec.args[1][0].equals(Var.create_safe("options"))
assert spec.args[1][1].equals(Var.create_safe(options))
assert (
format.format_event(spec)
== f'E("_remove_cookie", {{key:"testkey",options:{json.dumps(options)}}})'
Expand All @@ -225,10 +238,10 @@ def test_set_local_storage():
spec = event.set_local_storage("testkey", "testvalue")
assert isinstance(spec, EventSpec)
assert spec.handler.fn.__qualname__ == "_set_local_storage"
assert spec.args == (
("key", "testkey"),
("value", "testvalue"),
)
assert spec.args[0][0].equals(Var.create_safe("key"))
assert spec.args[0][1].equals(Var.create_safe("testkey"))
assert spec.args[1][0].equals(Var.create_safe("value"))
assert spec.args[1][1].equals(Var.create_safe("testvalue"))
assert (
format.format_event(spec)
== 'E("_set_local_storage", {key:"testkey",value:"testvalue"})'
Expand All @@ -249,5 +262,6 @@ def test_remove_local_storage():
spec = event.remove_local_storage("testkey")
assert isinstance(spec, EventSpec)
assert spec.handler.fn.__qualname__ == "_remove_local_storage"
assert spec.args == (("key", "testkey"),)
assert spec.args[0][0].equals(Var.create_safe("key"))
assert spec.args[0][1].equals(Var.create_safe("testkey"))
assert format.format_event(spec) == 'E("_remove_local_storage", {key:"testkey"})'
Loading

0 comments on commit 71811a6

Please sign in to comment.