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

Parameterized objects (e.g. Panel Widgets) as class attributes #832

Open
jbednar opened this issue Sep 14, 2023 · 3 comments
Open

Parameterized objects (e.g. Panel Widgets) as class attributes #832

jbednar opened this issue Sep 14, 2023 · 3 comments
Labels
type-feature Feature request

Comments

@jbednar
Copy link
Member

jbednar commented Sep 14, 2023

Param was designed to support nested Parameterized objects, where the value of a Parameter is a Parameterized that has its own Parameters:

import param, panel as pn

class P(param.Parameterized):
    x = param.Integer(5)
    y = param.ClassSelector(param.Parameterized, default=pn.widgets.IntSlider(value=7))

    @param.depends("y.value", watch=True)
    def fn(self):
        self.x = self.y.value

p = P()

p.y.value=32
p.x
# 32

Here the value of y is a Parameterized that happens to be a HoloViz Panel widget that has a Parameter named value, and you can see that Param code can depend on that widget's Parameters.

However, several Panel core developers have noticed Panel users writing classes like:

import param, panel as pn

class P(param.Parameterized):
    x = param.Integer(5)
    y = pn.widgets.IntSlider(value=7)

    @param.depends("y.value", watch=True)
    def fn(self):
        self.x = self.y.value

p = P()

p.y.value=32
p.x
# 32

This code was not anticipated when writing Param but it happens to work in this case, because the dependency code does not actually check that y was defined as a Parameter before fetching the value parameter from that object. And the code makes sense at a semantic level, because both a param.Integer and a pn.widgets.IntSlider are objects that have an underlying integer value, so it would seem reasonable to be able to use a widget the same way one uses a Parameter.

However, because this case was never intended to be supported, a widget used as if it were a Parameter does not actually work like a Parameter in all cases. In particular, the Parameter x will be instantiated into an object, giving the object an independent copy of the value for x, while the widget y is a normal Python class attribute, and will thus not be instantiated. So things will seem to be working, but if the class P is ever instantiated more than once, x will be independent per object, and y will be shared across all objects, leading to untold subtle bugs.

A user can currently avoid this issue by moving the widget definition out of the class and into the constructor:

import param, panel as pn

class P(param.Parameterized):
    x = param.Integer(5)

    def __init__(self, **params):
       super().__init__(**params)
       self.y = pn.widgets.IntSlider(value=7)

    @param.depends("y.value", watch=True)
    def fn(self):
        self.x = self.y.value

p = P()

p.y.value=21
p.x
# 5 # Why is this not 32?

But that's a lot wordier, fails to declare the Parameter value in the class's manifest of Parameters, and in this case doesn't work (maybe because of #829?).

Because people seem to fall into the trap of putting widgets at the class level quite a bit, I think we should do something about it. The options I can think of are:

  1. Warn people not to do this! If we find a Parameterized as a class attribute, and certainly if we find one being depended on, warn that this attribute will not behave like a true Parameter and will not be instantiated per object, and point people to an implementation like the first one above.

  2. If we find a Parameterized q as a class attribute of a Parameterized class, treat it as if there were a Parameter declared like param.ClassSelector(param.Parameterized, default=q). We probably wouldn't actually create such a Parameter, but we'd instantiate a copy of the Parameterized into each object as if we had. I.e., just accept that people will do this, and treat it as legal. In practice I'd guess it's only Panel users who expect such usage to work, but the implementation would be for any Parameterized, and maybe there are other similar applications for this approach.

  3. Under the theory that people are treating Panel Widgets and Parameters as interchangeable, one could imagine doing even more magic, so that if we find a Panel Widget (or e.g. use duck typing and find a Parameterized with a value Parameter), treat it as if they had actually declared that Parameter in this class (y=param.Integer(7) here, where y becomes a reference to the value parameter of this widget). It's hard to pin down how that could work, and it would be Panel-specific deep magic, so it would be hard to convince me that it's a good idea, but I wanted to list the idea concretely just so that we can be sure that what people seem to be assuming would work is really not something we should ever be supporting.

Personally, I vote against 3, and by default I'd vote for 1 because 2 doesn't solve a problem I have. But if 2 makes sense to other people and would support new users better, I don't think it would be difficult to implement, and I don't think it would affect code that's currently working properly. Any thoughts from others?

@jbednar jbednar added type-feature Feature request TRIAGE User-submitted issue that hasn't been triaged yet. labels Sep 14, 2023
@maximlt maximlt removed the TRIAGE User-submitted issue that hasn't been triaged yet. label Sep 14, 2023
@maximlt
Copy link
Member

maximlt commented Sep 14, 2023

I was surprised to see the second example working, I thought it worked only when y was set in the constructor. Indeed, there are some untold bugs, like a new watcher is mounted on the widget value every time a new instance is created.

image

Note that the example below actually raises an error currently, I believe due to #777. It would work if you set y before calling super().__init__ in the constructor. @philippjfr we need to make sure we're okay with this for Param 2?

import param, panel as pn

class P(param.Parameterized):
    x = param.Integer(5)

    def __init__(self, **params):
       super().__init__(**params)
       self.y = pn.widgets.IntSlider(value=7)

    @param.depends("y.value", watch=True)
    def fn(self):
        self.x = self.y.value

p = P()

p.y.value=21
p.x
# 5 # Why is this not 32?

An alternative to the above and that is somewhat less verbose is to leverage ClassSelector. It works well because the ClassSelector Parameter has instantiate=True by default so Param makes a deepcopy of the widget. Parameter(widget_instance, instantiate=True) would also work well, but that would require the users making the class variable mistake to understand instantiate, I wouldn't be on that, it's never been really understood well.

import param, panel as pn

class P(param.Parameterized):
    x = param.Integer(5)
    y = param.ClassSelector(default=pn.widgets.IntSlider(value=7), class_=pn.widgets.IntSlider)

    @param.depends("y.value", watch=True)
    def fn(self):
        print('callback')
        self.x = self.y.value

p = P()

p.y.value = 21
p.x

Among the suggestions, I would be more in favor of 1. Even though I think having a Parameterized instance as a class variable can be a valid thing to do, after all it's just a mutable data structure and there are valid use cases for such an object as a class variable. So if that were to be implemented, I'd make warning the default behavior but would allow disabling it altogether (import param; param.warn_parameterized_classvar = False). Or, the default behavior could be not to warn, but Panel would revert that.


This issue focused on Panel widgets, but I've seen users put all sorts of stuff as class variables while I assume that wasn't really their intention. Let's not forget that many Panel users aren't software developers. I remember clearly finding Param super magic when I started using it, how the hell can these Parameters that look like class variables be turned into instance variables?! That went against all the little Python knowledge I had at the time :)

So while there are ways we could improve Param's API to help our users, I also think there are documentation improvements we could make:

  • I looked for class variable in the Param documentation, there's no occurrence of that.
  • Panel How-To guides don't explain how to create a class (Parameterized or not) whose attributes are widgets, they're jumping directly from function/pn.bind to Parameterized class/mapping to widgets.
  • There's a reference to this issue in the Explanation page Panel and Param, it's pretty new and I'm not sure it's easily discoverable
  • Having both param.depends and pn.depends being used somewhat interchangeably probably lead some users to
    assume Panel objects and Parameters behave similarly.

I've put together a list of examples found on Discourse,

https://discourse.holoviz.org/t/why-do-some-panel-elements-dissappear-while-others-persist/1260: all sorts of objects set as class variables without any Parameter.

...
class Mapview(param.Parameterized):
    
    tiles         = gv.tile_sources.CartoEco()
    aoi_polygons  = gv.Polygons([], crs=crs.GOOGLE_MERCATOR)
    aoi_colours   = ['red','blue','green','orange','purple']
    aoi_stream    = hv.streams.PolyDraw(source=aoi_polygons, num_objects=5,styles={'fill_color': aoi_colours})
    template_df   = pd.DataFrame({'lng': [], 'lat': []}, columns=['lng', 'lat'])
    dfstream      = hv.streams.Buffer(template_df, index=False, length=10000, following=False)
    points        = hv.DynamicMap(gv.Points, streams=[dfstream])
    map_layout    = tiles * aoi_polygons * points

    def show_map(self):
        # set style options on map_layout
        self.map_layout.opts(
            # the ratio of WMTS height:width must be eqaul to the data_aspect value (0.5)
            # or the map will stretch/skew
            gv.opts.WMTS(global_extent=True,width=1200,height=600,show_grid=False,xaxis=None,yaxis=None),
            gv.opts.Polygons(fill_alpha=0.1),
            gv.opts.Points(size=5, color='green', fill_alpha=0.3, line_alpha=0.4)
        )        
        return self.map_layout.opts(data_aspect=0.5)

https://discourse.holoviz.org/t/how-to-access-param-values-of-widgets-inside-a-dynamically-defined-widgetbox/2024/5: WidgetBox layout as a class variable

...
class DynamicWidgetBox(param.Parameterized):
    options = param.ListSelector(default=[], objects=['a', 'b', 'c'])
    
    wBox = pn.WidgetBox(horizontal=True)
    
    @param.depends('options')
    def widgets(self):
        selects = []
        for v in self.options:
            selects.append(pn.widgets.MultiSelect(name=v, options=[1,2,3,4,5]))

        self.wBox[:] = [*selects]
        return self.wBox
...

https://discourse.holoviz.org/t/multiselect-widget-with-multiple-options-for-each-value/1118/3: data and a widget as class variable, with @pn.depends('widget.value') used to register a callback.

import panel as pn
import pandas as pd
import param 
import holoviews as hv
import hvplot.pandas

class AppTest(param.Parameterized):
    test_dict = {"key1" : ['A']*20,
                 "key2" : [1]*20,
                 "key3": [1,2]*10,
                 "value" : [100]*20}

    test_dtf = pd.DataFrame(test_dict)
    test_list = ['key1','key2','key3']
    default_value=test_dtf[test_list].drop_duplicates().values.tolist()[0]
    multi_select = pn.widgets.MultiSelect(name='Test', value=[default_value]   ,options= test_dtf[test_list].drop_duplicates().values.tolist())
    
    
    @pn.depends('multi_select.value')
    def view(self):
        if len(self.multi_select.value) ==0:
            return hv.Curve([])
        else:
            df=pd.DataFrame(self.multi_select.value,columns=['val','x','y'])
            return df.hvplot.scatter('x','y')

pn.extension()
viewer = AppTest()
pn.Row(viewer.multi_select,viewer.view)

https://discourse.holoviz.org/t/panel-tabs-updated-widget-values/1500/2: @pn.depends('widget.value') used to register a callback, watching multiple widgets.

import panel as pn
import param 
pn.extension()

class Tabs_example(param.Parameterized):
    intinput = pn.widgets.IntInput(value=0)
    txtinput = pn.widgets.TextInput(value='a')
    intslide = pn.widgets.IntRangeSlider(start=0,end=10)
    
    @pn.depends('intinput.value','txtinput.value','intslide.value')
    def summary(self):
        return pn.pane.Markdown(f'int : {self.intinput.value} <br> txt : {self.txtinput.value} <br> slide start :{self.intslide.value[0]} <br> slide end : {self.intslide.value[1]}')

t=Tabs_example()

pn.Tabs(('int',t.intinput),('text',t.txtinput),('slider',t.intslide),('summary',t.summary))

https://discourse.holoviz.org/t/simple-form-using-panel/5428/2: super().__init__ not called.

...
class Navbar(param.Parameterized):
    def __init__(self):
        self.left_nav_bar = pn.widgets.Select(
            name="Choose",
            options=[
                "Employees",
                "Customers",
            ],
        )
        self.customer_tabs = Customer()
        self.employee_tabs = Employee()


    @pn.depends("left_nav_bar.value")
    def display(self):
        if self.left_nav_bar.value == "Employees":
            return self.employee_tabs.display
        else:
            return self.customer_tabs.display
...

@jbednar
Copy link
Member Author

jbednar commented Sep 14, 2023

Yikes! Thanks for those examples "from the wild". That code will have all sorts of issues with some things being instance attributes and some things being shared across all instances; fun to watch what happens as long as you don't need it to work! Much of it does provide support for option 2: If people repeatedly and predictably expect Parameterized classes to handle Parameterized class attributes like Parameters, should we just do that? pd.DataFrame used as above would suggest an even more extreme Option 4 of treating all class attributes as Parameters. Supporting Option 4 seems likely to be a major limitation on how people can use a Parameterized class in a larger codebase, and in this case sharing a single df across instances is usually ok and actually desirable, so hopefully we don't need to consider 4.

An alternative to the above and that is somewhat less verbose is to leverage ClassSelector

Not sure which one you mean by "the above"; the code I showed as the way we expected it to be done already does use ClassSelector?

y = param.ClassSelector(default=pn.widgets.IntSlider(value=7), class_=pn.widgets.IntSlider)

To encourage this usage, should we make the class_ argument for ClassSelector optional and make it default to the type() of the default? That way people would just have to type y = param.ClassSelector(default=pn.widgets.IntSlider(value=7)).

I looked for "class variable" in the Param documentation, there's no occurrence of that.

I assume that's because in Python it's a class "attribute", not a class variable. We should maybe throw in "variable" in some places alongside "attribute" in case that's what people are searching for based on terminology from other languages.

@jbednar
Copy link
Member Author

jbednar commented Sep 22, 2023

Capturing discussion between @philippjfr and myself today, the ways we could think to address this issue are:

  1. Special Panel class for Panel users to use instead of Parameterized (e.g. adapting Viewable)
    • Pros: Can be very specific to Panel use cases without affecting any other use of Parameterized. Backwards compatibility issues likely to be very limited.
    • Cons: Won't address issues with people who do choose param.Parameterized (which we'd steer them away from if they are using widgets directly)
  2. Special behavior for param.Parameterized
    a. Duplicate Parameterizeds (or Panel objects, or Parameterized that have a "value") the same as we do Parameters, but don't make them Parameters
    - Pros: Can provide per-object instantiation without changing how a widget is accessed at the class level
    - Cons: Hard to reason about, accessing the "value" of the widget from within the class is wordy and nonobvious, unable to set any options for the Parameter that gets created (e.g. per_instance).
    b. Promote a Panel widget (or any Parameterized? or Parameterized that have a "value"?) to a ClassSelector(class=..widget, default=the widget) when found at the Parameterized class level
    - Pros: Simple syntax, ensures instantiation of widgets per instance.
    - Cons: Accessing the "value" of the widget from within the class is wordy and nonobvious, unable to set any options for the Parameter that gets created (e.g. per_instance).
    c. Create a Parameter for the value, then store the widget somewhere for recreation around the instantiated parameter.
    - Pros: value of the widget becomes accessible simply, as the Python attribute of the instance, with the same name as the Parameter
    - Cons: Hard to reason about; Probably breaks a whole lot of code.

Our general impression was that option 1 was the most appropriate, making this be a Panel issue rather than a Param one, but other input would be welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature Feature request
Projects
None yet
Development

No branches or pull requests

2 participants