Skip to content

Make it possible to resolve rx expressions recursively#918

Merged
philippjfr merged 2 commits into
mainfrom
resolve_rx
Mar 13, 2024
Merged

Make it possible to resolve rx expressions recursively#918
philippjfr merged 2 commits into
mainfrom
resolve_rx

Conversation

@philippjfr

@philippjfr philippjfr commented Mar 11, 2024

Copy link
Copy Markdown
Member

I'm a little afraid at how "meta" this operation is but it allows doing some things which aren't otherwise possible with .rx (or rather extremely complicated). Specifically this allows writing expressions that themselves contain references and then recursively resolves them. The case where I encountered the need for this is a Panel ToDo app.

Let us say we have a layout of tasks:

tasks = pn.Column(task1, task2)

Each task is structured like this:

task = Row(Checkbox(), Markdown())

Now I can write a simple expression to gather up all checkboxes:

checkboxes = tasks.param.objects.rx().rx.map(lambda task: task[0])

And if want to get the number of completed tasks I could do:

sum(checkboxes.rx.map(lambda w: w.value))

Unfortunately this only updates when the list of checkboxes changes, i.e. NOT when each individual checkbox is ticked or unticked.

What I really want to do is for the expression to resolve the references (i.e. in this case the widgets) and update when any of the references change. Using .rx.resolve I can count the number of completed tasks with:

sum(tasks.param.objects.rx().rx.map(lambda task: task[0]).rx.resolve())

Comment thread param/parameterized.py
return function(*args, **kwargs)

def resolve_value(value):
def resolve_value(value, recursive=True):

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was an oversight, it should not have recursively resolved values unless requested. Unfortunately changing the default behavior would cause potential regressions now since Panel and HoloViews both depend on it. So the migration plan will have to be for those libraries to explicitly set recursive=True/False and then eventually we can consider changing the default.

@philippjfr

Copy link
Copy Markdown
Member Author

Going to merge so I can cut an RC. If you have review comments please still leave them and I'll follow up before release.

@philippjfr philippjfr merged commit 8456e8b into main Mar 13, 2024
@philippjfr philippjfr deleted the resolve_rx branch March 13, 2024 11:48

@hoxbro hoxbro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have left some minor comments.

Comment thread param/reactive.py
refs = resolve_ref(self.object, nested)
value = resolve_value(self.object, nested)
if self.recursive:
new_refs = [r for r in resolve_ref(value, nested) if r not in refs]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
new_refs = [r for r in resolve_ref(value, nested) if r not in refs]
new_refs = [r for r in resolve_ref(value, recursive=nested) if r not in refs]

Is clearer for me to understand by using keywords arguments here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, although it's also a little confusing, recursive means something different in the context of resolve_ref than it does in the context of the Resolver.

Comment thread param/reactive.py
new_refs = [r for r in resolve_ref(value, nested) if r not in refs]
while new_refs:
refs += new_refs
value = resolve_value(value, nested)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
value = resolve_value(value, nested)
value = resolve_value(value, recursive=nested)

Comment thread param/reactive.py
while new_refs:
refs += new_refs
value = resolve_value(value, nested)
new_refs = [r for r in resolve_ref(value, nested) if r not in refs]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
new_refs = [r for r in resolve_ref(value, nested) if r not in refs]
new_refs = [r for r in resolve_ref(value, recursive=nested) if r not in refs]

Comment thread param/reactive.py
value = resolve_value(self.object, nested)
if self.recursive:
new_refs = [r for r in resolve_ref(value, nested) if r not in refs]
while new_refs:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
while new_refs:
while new_refs := [r for r in resolve_ref(value, recursive=nested) if r not in refs]:

And delete the last line in the while-loop. (I know you like these walrus)

Comment thread param/reactive.py
"""
return self._as_rx()._apply_operator(func, *args, **kwargs)

def resolve(self, nested=True, recursive=False):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def resolve(self, nested=True, recursive=False):
def resolve(self, *, nested=True, recursive=False):

So that we don't get a function call like this resolve(True, True)

Comment thread tests/testreactive.py
@philippjfr

Copy link
Copy Markdown
Member Author

Thanks, will make a new PR incorporating all your suggestions.

@Coderambling

Copy link
Copy Markdown
Contributor

Nice. Is the intention for this to be part of 1.4?

@philippjfr philippjfr mentioned this pull request Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants