Repository: kivy/kivy
Pull request: RecycleView: Add behavior to set RV data using kv ids
Adds the `RecycleKVIDsDataViewBehavior` class. Fixes #6878.

It's similar to the existing `RecycleDataViewBehavior` class, except that the data can signify properties of objects named with an id in KV. E.g. given a KV rule::

```
<MyRule@RecycleKVIDsDataViewBehavior+BoxLayout>:
    Label:
        id: name
    Label:
        id: value
```

Then setting the data list with `rv.data = [{'name.text': 'Kivy user', 'value.text': '12'}]` would automatically set the corresponding labels.

Problem statement:
Allow using kv ids in RecycleView data keys
**Software Versions**
* Python: 3.6.9 
* OS: Ubuntu 18.04.4 LTS
* Kivy: 1.11.0
* Kivy installation method: PIP

**Describe the bug**
The official information regarding recycleview shows the following: 
```yaml
<RV>:
    viewclass: 'Label' ### Here
    RecycleBoxLayout:
        default_size: None, dp(56)
        default_size_hint: 1, None
        size_hint_y: None
        height: self.minimum_height
        orientation: 'vertical'
```

Where viewclass is a simple Label widget with the attribute "text".  Created Label widgets within RV are populated as follows:

```python
class RV(RecycleView):
    def __init__(self, **kwargs):
        super(RV, self).__init__(**kwargs)
        self.data = [{'text': str(x)} for x in range(100)] ###### here one can see the class attribute 'text'
```

If one creates a more complicated widget for the viewclass within RecycleView as follows: 

```yaml
<MyRecycleView>:
    viewclass: 'ThisInfo' ### Mix of widgets
    RecycleBoxLayout:
        orientation: 'vertical'
        default_size: None, dp(120)
        default_size_hint: 1, None
        size_hint_y: None
        height: self.minimum_height
        spacing: 3

<ThisInfo@BoxLayout>:
    BoxLayout:
        size_hint_x: 1.3
        orientation: 'vertical'
        Label:
            id: name                 ### I should be able to modify this 'text' attribute from self.data by its id
        GridLayout:
            cols: 2
            Label:
                text: 'Position'
            Label:
                text: 'Salary'
            Label:
                id: position         ## Same here
            Label:
                id: salary           ### And here
```

And tries to populate in any of this ways, it does not populate any info, all the id fields appear blank.

```python
############### Method 1, placing the ID and attribute of the pertinent widget ###############
class MyRecycleView(RecycleView):
    def __init__(self,  **kwargs):
        super(MyRecycleView, self).__init__(**kwargs)

        self.data = [{'name.text': employee.name, 
                            'position.text': employee.position, 
                            'salary.text': employee.salary} 
                                 for employee in all_employees] 

######## Method 2, placing Id as the key of the widget and a dictionary for all of its attributes #####
class MyRecycleView(RecycleView):
    def __init__(self,  **kwargs):
        super(MyRecycleView, self).__init__(**kwargs)

        self.data = [{'name': {'text': employee.name}, 
                           'position': {'text': employee.position},
                           'salary': {'text':  employee.salary}} 
                               for employee in all_employees]
```

**Expected behavior**
The behavior is not consistent, when using a base class as Label, one may populate its attributes directly in the self.data list with 'text'. When using a mixture of widgets with separate ids one should be able to populate its attributes by its ID just as in all the rest of the kivy framework.

Hints from issue discussion:
that's interesting, my way of achieving this is to map the properties of the internal widgets to properties on the root widget, and populate them there.

```yaml
<ThisInfo@BoxLayout>:
    name_text: ""
    salary_text: ""
    position_text: ""
    BoxLayout:
        size_hint_x: 1.3
        orientation: 'vertical'
        Label:
            text: root.name_text or ""
        GridLayout:
            cols: 2
            Label:
                text: 'Position'
            Label:
                text: 'Salary'
            Label:
                text: root.position_text or ""
            Label:
                text: root.salary_text or ""
```

and in python
```python
        self.data = [{'name_text': employee.name, 
                           'position_text': employee.position,
                           'salary_text':  employee.salary} 
                               for employee in all_employees]
```

Feel free to propose a pull request if you want your proposed way to work, but as you can see the current way allow filling the need pretty easily, while (imho) defining the interface of the widget more clearly.
I pretty much agree with what @tshirtman said and that has also been my workaround. 

I would probably reject such a PR though tbh, because it would complicate the rv code. Right now it's basically just a `setattr` in a loop, but this would add needles complexity for the most part, when there's a simple workaround.

A PR that would be more acceptable, is perhaps providing subclasses of `RecycleDataViewBehavior` and `RecycleDataViewBehavior` that do this. That way people can "opt-in" to this behavior and that would probably be a net improvement. Perhaps I may even work on this, if no one else does.
I've used that work around before, and I believe that is the problem, it feels like a work around and not the optimal way to do it. 

Programming is all about logic and the logic thing to do in the scenario shared would be to populate the objects attributes directly following kivy's logic of programming, instead of creating aux variables, populate them and then use them to populate the objects attributes. 
https://github.com/kivy/kivy/pull/6897 implements this feature. See the updated example for how to use it. @tshirtman

Commit message: Add back root data key to example.

Patch to review:
diff --git a/examples/widgets/recycleview/basic_data.py b/examples/widgets/recycleview/basic_data.py
index 94071252d4..41bd281591 100644
--- a/examples/widgets/recycleview/basic_data.py
+++ b/examples/widgets/recycleview/basic_data.py
@@ -1,4 +1,4 @@
-from random import sample
+from random import sample, randint
 from string import ascii_lowercase
 
 from kivy.app import App
@@ -7,7 +7,7 @@
 
 
 kv = """
-<Row@BoxLayout>:
+<Row@RecycleKVIDsDataViewBehavior+BoxLayout>:
     canvas.before:
         Color:
             rgba: 0.5, 0.5, 0.5, 1
@@ -15,6 +15,8 @@
             size: self.size
             pos: self.pos
     value: ''
+    Label:
+        id: name
     Label:
         text: root.value
 
@@ -88,21 +90,24 @@
 class Test(BoxLayout):
 
     def populate(self):
-        self.rv.data = [{'value': ''.join(sample(ascii_lowercase, 6))}
-                        for x in range(50)]
+        self.rv.data = [
+            {'name.text': ''.join(sample(ascii_lowercase, 6)),
+             'value': str(randint(0, 2000))}
+            for x in range(50)]
 
     def sort(self):
-        self.rv.data = sorted(self.rv.data, key=lambda x: x['value'])
+        self.rv.data = sorted(self.rv.data, key=lambda x: x['name.text'])
 
     def clear(self):
         self.rv.data = []
 
     def insert(self, value):
-        self.rv.data.insert(0, {'value': value or 'default value'})
+        self.rv.data.insert(0, {
+            'name.text': value or 'default value', 'value': 'unknown'})
 
     def update(self, value):
         if self.rv.data:
-            self.rv.data[0]['value'] = value or 'default new value'
+            self.rv.data[0]['name.text'] = value or 'default new value'
             self.rv.refresh_from_data()
 
     def remove(self):
diff --git a/kivy/factory_registers.py b/kivy/factory_registers.py
index 6efe38bc39..bc9a4a4a26 100644
--- a/kivy/factory_registers.py
+++ b/kivy/factory_registers.py
@@ -182,6 +182,7 @@
 r('LayoutSelectionBehavior', module='kivy.uix.recycleview.layout')
 r('RecycleLayoutManagerBehavior', module='kivy.uix.recycleview.layout')
 r('RecycleDataViewBehavior', module='kivy.uix.recycleview.views')
+r('RecycleKVIDsDataViewBehavior', module='kivy.uix.recycleview.views')
 r('RecycleDataAdapter', module='kivy.uix.recycleview.views')
 r('RecycleDataModelBehavior', module='kivy.uix.recycleview.datamodel')
 r('RecycleDataModel', module='kivy.uix.recycleview.datamodel')
diff --git a/kivy/uix/recycleview/views.py b/kivy/uix/recycleview/views.py
index 0174027d38..538c39bff9 100644
--- a/kivy/uix/recycleview/views.py
+++ b/kivy/uix/recycleview/views.py
@@ -17,11 +17,14 @@
 
 '''
 
-from kivy.properties import StringProperty, ObjectProperty
+from kivy.properties import ObjectProperty
 from kivy.event import EventDispatcher
-from kivy.factory import Factory
 from collections import defaultdict
 
+__all__ = (
+    'RecycleDataViewBehavior', 'RecycleKVIDsDataViewBehavior',
+    'RecycleDataAdapter')
+
 _view_base_cache = {}
 '''Cache whose keys are classes and values is a boolean indicating whether the
 class inherits from :class:`RecycleDataViewBehavior`.
@@ -108,6 +111,34 @@ def apply_selection(self, rv, index, is_selected):
         pass
 
 
+class RecycleKVIDsDataViewBehavior(RecycleDataViewBehavior):
+    """Similar to :class:`RecycleDataViewBehavior`, except that the data keys
+    can signify properties of objects named with an id in KV.
+
+    E.g. given a KV rule::
+
+        <MyRule@RecycleKVIDsDataViewBehavior+BoxLayout>:
+            Label:
+                id: name
+            Label:
+                id: value
+
+    Then setting the data list with
+    ``rv.data = [{'name.text': 'Kivy user', 'value.text': '12'}]`` would
+    automatically set the corresponding labels.
+    """
+
+    def refresh_view_attrs(self, rv, index, data):
+        sizing_attrs = RecycleDataAdapter._sizing_attrs
+        for key, value in data.items():
+            if key not in sizing_attrs:
+                name, *ids = key.split('.', maxsplit=1)
+                if ids:
+                    setattr(self.ids[name], ids[0], value)
+                else:
+                    setattr(self, name, value)
+
+
 class RecycleDataAdapter(EventDispatcher):
     '''The class that converts data to a view.
 


Code review comments:
diff_hunk: @@ -108,6 +111,34 @@ def apply_selection(self, rv, index, is_selected):
         pass
 
 
+class RecycleKVIDsDataViewBehavior(RecycleDataViewBehavior):
+    """Similar to :class:`RecycleDataViewBehavior`, except that the data keys
+    can signify properties of objects named with an id in KV.
+
+    E.g. given a KV rule::
+
+        <MyRule@RecycleKVIDsDataViewBehavior+BoxLayout>:
+            Label:
+                id: name
+            Label:
+                id: value
+
+    Then setting the data list with
+    ``rv.data = [{'name.text': 'Kivy user', 'value.text': '12'}]`` would
+    automatically set the corresponding labels.
+    """
+
+    def refresh_view_attrs(self, rv, index, data):
+        sizing_attrs = RecycleDataAdapter._sizing_attrs
+        for key, value in data.items():
+            if key not in sizing_attrs:
+                name, *ids = key.split('.', maxsplit=1)
path: kivy/uix/recycleview/views.py
text: @user1:
why the star? the maxsplit makes it a 2-tuple, so we should be able to directly use the value without indexing on the next line.

Also, i think like `ids` is not a great name here, as `name` actually refer to a kv id, and `ids` here would be a chain of one or more attributes.
And why the maxsplit actually? Do we really want to set an attribute with dots in it? wouldn't it be better to iterate on the names, getting the next value until the penultimate one and use setattr on this one?

```python
wid, *attrs = key.split('.')
obj = self
for attr in attrs[:-1]:
    obj = getattr(obj, attr)
setattr(obj, attrs[-1], value)
```
or would that cause issues i can't think of right now?

@author:
>why the star? the maxsplit makes it a 2-tuple, so we should be able to directly use the value without indexing on the next line.

Because the `maxsplit` is 1 we can have **at most** 2 items. But it could also be only 1. In the latter case, `ids` will be empty.

@author:
>Also, i think like ids is not a great name here, as name actually refer to a kv id, and ids here would be a chain of one or more attributes.

I feel like that's nitpicking because in the normal case it is the property name, except when it's interpreted as an id :) But also, I think you may have misunderstood what this is doing. Specifically, with the following KV examples:

```
MyRule:
    street: ''
    Label:
        id: address

<RootRule@user2>:
    country: ''
    Label:
        id: phone
    MyRule:
        id: my_rule
```

Setting `data` to `[{'country': 'US', 'phone.text': '123456', 'my_rule.street': 'Home ave', 'my_rule.address.text': '555'}]`. Under the current code, `my_rule` will have a attribute named `address.text` created and set to `555`. That's because we don't recursively go through rules like your example code. We only allow accessing the `ids` at the first level. Hence the only 1 split by period. Under your code, it would set the `text` property of the `address` widget to `555`.

The reason is that I don't think it would be common in kivy to set properties this way for sub rules and sub-sub rules etc. So I don't think we should complicate the API by adding this. Because once we add that it's hard to remove such functionality and I'd prefer to wait until this specifically is requested. And even then I think we can just add such recursive `ids` access a doc recipe. For now, this API only allows setting the property of a widget named in the root rule of the class.

@user1:
ok, i did think a recursive definition could be useful, but i agree it might be a strong commitment that we might not need to make.

> Under the current code, my_rule will have a attribute named address.text created and set to 555

I think that's what i meant with "Do we really want to set an attribute with dots in it?", that seems like a weird requirement to me, and i think i'd rather produce an error, saying that recursive setting (what i would expect by putting dots in the key) is not supported, than create attributes with dots in them, that can only be accessed through getattr, and won't properly bind in kvlang.

@author:
I suppose we can raise an exception if there is more than one dot, in case users think that it does work recursively.

@author:
Ok, I added that.