holoviz / param

Param: Make your Python code clearer and more reliable by declaring Parameters
https://param.holoviz.org
BSD 3-Clause "New" or "Revised" License
434 stars 74 forks source link

ObjectSelect objects key, value changes to value, key after update #398

Closed MarcSkovMadsen closed 1 year ago

MarcSkovMadsen commented 4 years ago

My Pain

I'm trying to dynamically update the objects from which a user can select. But the behavior of param.ObjectSelector.objects is different before and after update.

$ python 'issue_object_selector.py'
Traceback (most recent call last):
  File "issue_object_selector.py", line 12, in <module>
    app.value=3
  File "C:\Users\masma\source\repos\orsteddcsmarketsanalytics\api\.venv\lib\site-packages\param\parameterized.py", line 294, in _f
    instance_param.__set__(obj, val)
  File "C:\Users\masma\source\repos\orsteddcsmarketsanalytics\api\.venv\lib\site-packages\param\parameterized.py", line 296, in _f
    return f(self, obj, val)
  File "C:\Users\masma\source\repos\orsteddcsmarketsanalytics\api\.venv\lib\site-packages\param\parameterized.py", line 823, in __set__
    self._validate(val)
  File "C:\Users\masma\source\repos\orsteddcsmarketsanalytics\api\.venv\lib\site-packages\param\__init__.py", line 1242, in _validate
    "valid options include %s"%(val,attrib_name, items))
ValueError: 3 not in Parameter value's list of possible objects, valid options include [Third, Fourth]
import param
OBJECTS_ALL = {"First":1 , "Second": 2}

class App(param.Parameterized):
    value = param.ObjectSelector(default=1, objects=OBJECTS_ALL)

app=App()
app.value=2

app.param.value.objects = {"Third": 3, "Fourth": 4}
app.value=3
MarcSkovMadsen commented 4 years ago

This is also strange. WHY DOES THE DICT BECOME A LIST OF CONSTRUCTION?

$ python 'issue_object_selector.py'
[1, 2]
2
{'Third': 3, 'Fourth': 4}
Third
(.venv)
import param
import panel as pn
OBJECTS_ALL = {"First":1, "Second": 2}

class App(param.Parameterized):
    value = param.ObjectSelector(default=1, objects=OBJECTS_ALL)

    @param.depends("value", watch=True)
    def update(self):
        print(self.param.value.objects)
        print(self.value)

app=App()
app.value=2

app.param.value.objects = {"Third": 3, "Fourth": 4}
app.value="Third"
MarcSkovMadsen commented 4 years ago

Ahh. I can see on construction of ObjectSelector that it changes the objects dict to a list

def __init__(self,default=None,objects=None,instantiate=False,
                 compute_default_fn=None,check_on_set=None,allow_None=None,**params):
        if objects is None:
            objects = []
        if isinstance(objects, collections_abc.Mapping):
            self.names = objects
            self.objects = list(objects.values())
        else:

The question is then how would you expect a user to change the objects dynamically in the program?

For example if you have a region ObjectSelector and you change the value then I would expect the country ObjectSelector objects to only be the countries in that region?

MarcSkovMadsen commented 4 years ago

Workaround

A workaround is

import param
OBJECTS_ALL = {"First":1, "Second": 2}

class App(param.Parameterized):
    value = param.ObjectSelector(default=1, objects=OBJECTS_ALL)

    @param.depends("value", watch=True)
    def update(self):
        print(self.param.value.objects)
        print(self.value)

app=App()
app.value=2

object_dict = {"Third": 3, "Fourth": 4}
objects = list(object_dict.values())
app.param.names = object_dict
app.param.value.objects = objects
app.value=3
$ python 'issue_object_selector.py'
[1, 2]
2
[3, 4]
3
(.venv)
philippjfr commented 4 years ago

Yeah, we should really make sure ObjectSelector supports dict as objects instead of converting in the constructor.

MarcSkovMadsen commented 4 years ago

By the way @philippjfr . The one thing I don't like about param is actually the order of the key, value in the objects dict. Its simply the opposite of what I normally see.

I would expect

objects = {1: "Denmark", 2: "Germany", ....}

not

objects = {"Denmark": 1, "Germany": 2}

I'm simply confused every time I use the ObjectSelector.

MarcSkovMadsen commented 4 years ago

Please Consider Creating a new Selector that fixes both these problems. I guess there are a lot of dependencies out there unfortunately.

philippjfr commented 4 years ago

Please Consider Creating a new Selector that fixes both these problems. I guess there are a lot of dependencies out there unfortunately.

Sadly this is very unlikely, we already added a param.Selector and I doubt we'll add another one again. I think the change to support a dictionary objects can be done without breaking backward compatibility. I agree that the key: value ordering is weird, but I think it was deliberate to mostly get around the problem of supporting only hashable objects (this is a huge source of pain in Panel's Select/MultiSelect widgets). You can see the relevant PRs here:

https://github.com/holoviz/param/pull/258 https://github.com/holoviz/param/pull/316

MarcSkovMadsen commented 4 years ago

There is a user reporting the same problem here https://github.com/holoviz/param/issues/331

kcpevey commented 4 years ago

Oddly enough, I'm still struggling with the same issues this week! I've hit all the things discussed above, but I had issues boiling them down to reproducible examples and explanations. Thank you for reporting!

jbednar commented 4 years ago

To help understand the underlying (and as far as I can see unsolvable) problem here, what's happening is a conflict between a Param-centric view of the world and a Panel-centric view of the world. Actually, it's more generally a conflict between Python-centric and GUI-centric worldviews.

For normal Python usage, a param.Parameter is fundamentally a Python attribute with a certain value stored in the attribute dictionary of an object. E.g. an Integer parameter x of object a might have a value of 3, such that a.x==3 and you can set it with a.x=2. The same is true if x is of a Selector type given a list allowing [1,2,3]; again a.x==3 and you can set it with a.x=2; Selector is just a more constrained version of Integer in this case. Fundamentally, Selectors aren't any different from any other Parameter type; they let you set a value (the actual item you want to work with in Python), and only intervene if you set it to a value not allowed by the type. The design of Param is that the same code should still run if you simply delete the Parameter declaration and simply access that attribute directly, in normal cases.

That's the view from Param in Python code, so what about for a GUI? For objects with a straightforward textual representation (strings, integers, objects with a "name" parameter), GUI support is easy, because we can inspect the current parameter value and all allowable values, construct text labels for all those, let the user select between the text labels, and set the underlying values. No problem -- going back and forth between labels and objects is easy in that case.

But what about other objects that have no inherent name like lists, arbitrary Python class instances, dicts, and other unhashable items? Now we have a problem. Sure, we can ask the user to provide explicit text labels for each object, which is what Selector does in that case. But now what do we do when we start up the GUI and need to show the current state, from reading out the value of that attribute? Fundamentally the current attribute state is a value, not a label, so we have to somehow look up the label (e.g. find 1, for a value we encounter of "Denmark"). The same is true if we want the GUI to reflect a value changed in Python while the GUI runs; again the GUI must map from values to labels, which is precisely the opposite of how a GUI author thinks. The GUI author thinks of mapping from labels to objects, because the GUI operations all start with labels. But in Python, the parameter is never accessed or set by label, it is set directly as a value. Labels are an afterthought or a hint for printing, and have nothing to do with the attribute value.

Hypothetically, if we were to create a new Selector type that worked the other way, users would no longer be able to access the value from Python directly. E.g. if it worked like objects = {1: "Denmark", 2: "Germany", ....}; the value of that Parameter would have to be 1, not "Denmark", and a user would need to explicitly use a lookup table to get "Denmark" every time they access the value. Such a Parameter could be supported, but it would be entirely unlike any other Parameter, and would only really make sense for GUI programming. In Python no one wants to find that the country is 1; they want to find that the country is "Denmark". Unlike Panel, Param is designed for GUI support to be an afterthought or a bonus, with actual Python usage of the attribute always being the primary consideration. Such a Parameter would be the only one breaking that rule, and I guarantee Python users would now come back and say that they are deeply confused about why this one Parameter behaves this way.

Hopefully you can see why we're having this problem -- we can't make it straightforward for the GUI (where labels are meaningful and real) without destroying its use as a normal Python attribute, and since Param (as opposed to Panel) is fundamentally not a GUI library, we have to side with Python usage, even when it makes GUI usage more confusing. In general the Python-centric and GUI-centric views are quite compatible (or else using Param for building GUIs wouldn't work!), but in this case there is a direct and unavoidable conflict; only one of those two viewpoints can be supported properly.

MarcSkovMadsen commented 4 years ago

Thanks @jbednar for taking the time.

I will have to spend a little time to digest and maybe respond. Have a feeling I actually don't agree.

But maybe I just need more time to understand/ test it. :-)

MarcSkovMadsen commented 4 years ago

A user is experiencing this problem here https://discourse.holoviz.org/t/replacing-options-dictionary-doesnt-replace-keys-choices-as-expcected/1244/6

jbednar commented 4 years ago

Opened a possible PR: https://github.com/holoviz/param/pull/440