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

Implement support for allowing Parameter references #843

Merged
merged 20 commits into from
Sep 24, 2023
Merged

Implement support for allowing Parameter references #843

merged 20 commits into from
Sep 24, 2023

Conversation

philippjfr
Copy link
Member

@philippjfr philippjfr commented Sep 24, 2023

This PR brings an exceptionally useful feature that we have had in Panel since 1.2 to Param. Specifically it allows passing Parameter references (i.e. Parameter objects, reactive functions and expressions) as a parameter value and then automatically mirrors the value of the reference, e.g.:

class Parameters(param.Parameterized):

    string = param.String(default="string", allow_refs=True)
 
p = Parameters()
p2 = Parameters(string=p.param.string)

assert p.string == p2.string
p.string = 'new_string'
assert p2.string == 'new_string'

This is much, much easier than setting up callbacks and together with the addition of reactive functions and expressions makes it super easy to set up parameters that depend on some other dynamically computed values. The implementation here is fully backward compatible, since you must enable the support for resolving references like this explicitly with the new Parameter.allow_refs slot.

Note that references can only be used on instances, classes will not attempt to resolve references, however you may use a class parameter as a reference.

This implementation also implements updating of references, which could not reasonably be implemented in Panel and is a good reason to move this support to Param.

In addition to allowing simple references to be resolved support can also be enabled for nested references using the Parameter.nested_refs slot. This allows constructs such as this to work:

class Parameters(param.Parameterized):

    string = param.String(default="string", allow_refs=True)

    dictionary = param.Dict(default={}, allow_refs=True, nested_refs=True)

p = Parameters()
p2 = Parameters(dictionary={'key': p.param.string})
p3 = Parameters(string='foo')

assert p2.dictionary == {'key': 'string'}
p2.dictionary = {'new key': p3.param.string}
assert p2.dictionary == {'new key': 'foo'}

param/parameterized.py Outdated Show resolved Hide resolved
Copy link
Member

@jbednar jbednar left a comment

Choose a reason for hiding this comment

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

I haven't used references in Panel code, so I'm not the best person to review this, but it looks reasonable to me. Having to declare nested references seems ok to me, since it's difficult to reason about all the implications involved, but having to declare a simple reference to be allowed seems like it will greatly limit how useful this is, because most people won't add that keyword and those that do will make their codebase get polluted with a lot of allow_refs=True. My inclination is allow_refs=True and nested_refs=False, but I don't know how likely that is to cause problems with current code.

@philippjfr
Copy link
Member Author

philippjfr commented Sep 24, 2023

I'm a little wary about changing that this close to release. How would you feel about the idea that in 2.0 we begin warning people that they should explicitly set allow_refs=False when we detect something that would be treated as a reference once we explicitly enable it. Then we can give people some time to update their code and then flip the switch by default in 2.1.

@jbednar
Copy link
Member

jbednar commented Sep 24, 2023

Ok, though I'd probably not announce 2.0 widely until 2.1 is in place, leaving the 2.0 as a helpful interim step for people to move their code into compatibility, rather than one we'd expect people to adopt widely. 2.0 already has compatibility implications and I'd like to guide people from where they are into the new way, rather than make them adapt twice.

param/parameterized.py Show resolved Hide resolved
param/parameterized.py Outdated Show resolved Hide resolved
@philippjfr
Copy link
Member Author

I'm going to go ahead and merge. As I need this in a dev release asap the timelines here are tight and this was written with backward compatibility at the forefront. If you're coming at this later please still give it a thorough review.

@philippjfr philippjfr merged commit 00dc733 into main Sep 24, 2023
10 checks passed
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