holoviz / panel

Panel: The powerful data exploration & web app framework for Python
https://panel.holoviz.org
BSD 3-Clause "New" or "Revised" License
4.74k stars 516 forks source link

Support specifying the columns of a Perspective pane as an iterable #2055

Open MarcSkovMadsen opened 3 years ago

MarcSkovMadsen commented 3 years ago

Panel 0.11:

If I run the below python code it raises an ValueError: List 'columns' must be a list..

import panel as pn
pn.extension("perspective")
import pandas as pd

data = {"b": ['1981 01 01 00   161    28 10173   270    21     0     0     0',
  '1981 01 01 01   111    33 10175   270    21     0     0 -9999',
  '1981 01 01 02   111    39 10183   330    26     0     0 -9999',
  '1981 01 01 03    94    39 10192 -9999     0     0     0 -9999',
  '1981 01 01 04    72    39 10196 -9999     0     0     0 -9999'],
 'a': [1981, 1981, 1981, 1981, 1981],
 }
df=pd.DataFrame(data)

pn.pane.Perspective(df, columns=df.columns).servable()
ValueError: List 'columns' must be a list.

Traceback (most recent call last):
  File "c:\repos\load-forecasts\.venv\lib\site-packages\bokeh\application\handlers\code_runner.py", line 197, in run
    exec(self._code, module.__dict__)
  File "C:\repos\load-forecasts\test_error.py", line 14, in <module>
    pn.pane.Perspective(df, columns=df.columns).servable()
  File "c:\repos\load-forecasts\.venv\lib\site-packages\panel\pane\base.py", line 111, in __init__
    super().__init__(object=object, **params)
  File "c:\repos\load-forecasts\.venv\lib\site-packages\panel\reactive.py", line 593, in __init__
    super().__init__(**params)
  File "c:\repos\load-forecasts\.venv\lib\site-packages\panel\reactive.py", line 71, in __init__
    super().__init__(**params)
  File "c:\repos\load-forecasts\.venv\lib\site-packages\panel\viewable.py", line 509, in __init__
    super().__init__(**params)
  File "c:\repos\load-forecasts\.venv\lib\site-packages\panel\viewable.py", line 370, in __init__
    super().__init__(**params)
  File "c:\repos\load-forecasts\.venv\lib\site-packages\panel\viewable.py", line 217, in __init__
    super().__init__(**params)
  File "c:\repos\load-forecasts\.venv\lib\site-packages\param\parameterized.py", line 2522, in __init__
    self.param._setup_params(**params)
  File "c:\repos\load-forecasts\.venv\lib\site-packages\param\parameterized.py", line 1065, in override_initialization
    fn(parameterized_instance, *args, **kw)
  File "c:\repos\load-forecasts\.venv\lib\site-packages\param\parameterized.py", line 1313, in _setup_params
    setattr(self, name, val)
  File "c:\repos\load-forecasts\.venv\lib\site-packages\param\parameterized.py", line 318, in _f
    return f(self, obj, val)
  File "c:\repos\load-forecasts\.venv\lib\site-packages\param\parameterized.py", line 892, in __set__
    self._validate(val)
  File "c:\repos\load-forecasts\.venv\lib\site-packages\param\__init__.py", line 1381, in _validate
    raise ValueError("List '%s' must be a list."%(self.name))
ValueError: List 'columns' must be a list.

Solution

Support specifying the columns as an iterable.

Additional Context

The reason why I specify the columns is that I want to preserve the column order of the dataframe. When using Perspective they are ordered alphabetically.

hoxbro commented 3 years ago

If you want to preserve the order of columns (right now) you can do this

pn.pane.Perspective(df, columns=list(df.columns)).servable()
hoxbro commented 3 years ago

Just to be clear - I think it should be a possibility to add an iterable as a column input.

A way to do this is replace param.List with the following for the column parameter. I don't know if there is a more correct way to do this but this seems the easiest to implement.

class ListLike(param.List):
    def _validate(self, val):
        if hasattr(val, "__iter__"):
            val = list(val)
        return super()._validate(val)
hoxbro commented 3 years ago

What is your opinion of this @philippjfr?

philippjfr commented 3 years ago

Not sure, maybe we should add an option to param.List and param.Tuple to accept iterables and automatically cast them to the declared type rather than introducing new parameters. @jbednar any thoughts?

MarcSkovMadsen commented 3 years ago

What would be the cons or risks of doing what you suggest philippjfr?

philippjfr commented 3 years ago

Well I wouldn't change the default behavior so we'd still have to update the param.List definitions in Panel where appropriate.

MarcSkovMadsen commented 3 years ago

The only one I see is that param.List and param.Tuple would not hold the instance it was initially provided. That might be confusing for some use cases?

philippjfr commented 3 years ago

That's definitely true, not so important for param.Tuple since tuples are immutable but I can imagine a user trying to modify the list the passed to a parameter inplace. I wouldn't say that's a recommended workflow though.

jbednar commented 3 years ago

Good question. In general it seems safe for Param to cast scalars to the indicated type (as recently proposed for Numpy integer scalars), but there is a large potential for confusion when converting mutable types, where the user could later change them and expect that value to be present in the Parameter value.

Would it be sufficient if we raised a more explicit error message in this case? I.e., check if the argument is iterable and if so tell the user to convert it to a list first? That way the onus is on the user to decide whether it's safe to cast it.

philippjfr commented 3 years ago

Honestly I'm kind of in the camp that either you should never modify mutable types held by a param inplace (and we should automatically wrap them in an equivalent immutable subclass) or we should provide mutable wrappers which notify panel if they have been modified. Because right now you can modify mutables and param.watch callbacks will have no idea, which frequently trips up users.

jbednar commented 3 years ago

I don't think it's that clear. E.g. Topographica, for which Param was built, had many objects with Parameter values that were Python class instances, and in such cases it's only making use of the identity of the object, not the current state, and nothing needs to be triggered at all when that state changes; the parameter value is just a selection of one object out of the many available objects. The same can be true of a param.List, which might specify some subset of a set of mutable Python objects. But I suppose in this case you'd want a notification when the list changes, not when one of the items in the list changes? If so, yes, that might be nice, but I don't know how to achieve it. I do know that automatically coercing some object into a list won't make that any easier!

philippjfr commented 3 years ago

Totally agree that there are varying needs for different projects here, although I'd argue Panel param.watch based usage now far exceeds the number of original users which preceded support for that.

If so, yes, that might be nice, but I don't know how to achieve it.

Bokeh uses list/dict subclasses which have mutation observers to achieve this.

jbednar commented 3 years ago

Totally agree that there are varying needs for different projects here, although I'd argue Panel param.watch based usage now far exceeds the number of original users which preceded support for that.

Sure, but I don't think selecting one of a finite set of mutable objects is a rare case even now, and would not normally mean we'd want to trigger on a mutation to that object. But triggering on the list or dict itself changing does indeed seem like a common need.