-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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: Optionally comparing fields in Var.contains, e.g. on rx.Base based types. #3375
feat: Optionally comparing fields in Var.contains, e.g. on rx.Base based types. #3375
Conversation
Leave the current implementation as is. Added test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, looks good to me. Thanks, @abulvenz
…-using-object-field-comparison
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i haven't tested the suggested change, but i think field should be specifiable as another var, like the other var operations.
reflex/vars.py
Outdated
_var_name = ( | ||
f"{self._var_name}.includes({other._var_full_name})" | ||
if field is None | ||
else f"{self._var_name}.some(e=>e.{field}==={other._var_full_name})" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this should work if the field is specified as a state var, so maybe we can write it like
field = Var.create_safe(field, _var_is_string=isinstance(field, str))
and
f"{self._var_name}.some(e=>e[{field._var_full_name}]==={other._var_full_name})"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@masenf Thanks for your idea. I tried it as you suggested:
_var_name = None
if field is None:
_var_name = f"{self._var_name}.includes({other._var_full_name})"
else:
field = Var.create_safe(field, _var_is_string=isinstance(field, str))
_var_name = f"{self._var_name}.some(e=>e[{field._var_full_name}]==={other._var_full_name})"
The code results in this for string literals passed in
<RadixThemesCheckbox
checked={state__state.selected_items.some(e=>e[key]===item.key)}
...
/>
where key is not defined ⚡.
Maybe it would be helpful for me to understand the aim of your suggested change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the purpose is being able to write something like
rx.foreach(
State.available_items,
lambda item: rx.menu.item(
rx.checkbox(
item.name,
checked=State.selected_items.contains(item.key, State.filter_key),
on_change=lambda _: State.select_item(item),
)
),
)
where you can change the value of filter_key
dynamically
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah okay, we actually need _var_name_unwrapped
, like this
_var_name = f"{self._var_name}.some(e=>e[{field._var_name_unwrapped}]==={other._var_full_name})"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @masenf, I will use your corrected version, it works with both strings and vars.
One thing that is that e.{field}
could also take nested values like field="address.street"
which will not work with the e[...]
syntax.
I can also support this case, when I use this abomination:
_var_name = f"{self._var_name}.some(e=>{field._var_name_unwrapped}.split(\".\").reduce((acc,v)=>acc[v],e)==={other._var_full_name})"
class ComplexKey(rx.Base):
part1: str
part2: int
class ComplexItem(rx.Base):
name: str
color: str
key: ComplexKey
Then you could write:
State.selected_items.contains(item.key.part1, "key.part1"),
But at some point it becomes ridiculously complex...
-------- 8< --------- 8< --------------
And ideas for sleepless nights 🛌 🖥️
Please nobody take this seriously 😄
import reflex.js as js
...
checked=State.selected_items.some(
js.λ("e",
js.var("e")
.subscript(State.filter_key)
.equals(js.var(State.search_text)
)
)
),
...
assert js.λ("e",
js.var("e")
.subscript(State.filter_key)
.equals(State.search_text)
)
).__str__() == "e=>e[state__state.filter_key]===state__state.search_text"
assert is_turing_complete(js)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🏆
…-using-object-field-comparison
…-using-object-field-comparison
…sed types. (reflex-dev#3375) * feat: Optionally comparing fields, e.g. on rx.Base based types. * feat: Minimally invasive change. Leave the current implementation as is. Added test. * fix: Supporting old-school python versions. * fix: Adding masenf's suggestions to use var instead of string.
I want to make the example app at the bottom work.
TLDR:
The
contains
function checks if a var is contained in the array by comparing a specific field.This can also be helpful in other circumstances.
Other implementation ideas:
_.isEqual
item.key
is passed (Maybe interfering with other use cases).selected_items
, that is not always possible (e.g. when using sqlalchemy-ORM objects.Screenshot of the app: