rawpython / remi

Python REMote Interface library. Platform independent. In about 100 Kbytes, perfect for your diet.
Apache License 2.0
3.52k stars 404 forks source link

Table API #154

Open fpp-gh opened 7 years ago

fpp-gh commented 7 years ago

Next subject is the Table widget. Below is the present skeleton API :

class Table(Widget):
    """ table widget - it will contains TableRow """
    @decorate_constructor_parameter_types([])
    def __init__(self, **kwargs):
        """ Args: kwargs: See Widget.__init__()"""

    def from_2d_matrix(self, _matrix, fill_title=True):
        """ Fills the table with the data contained in the provided 2d _matrix
        The first row of the matrix is set as table title """

class TableRow(Widget):
    """ row widget for the Table - it will contains TableItem """
    @decorate_constructor_parameter_types([])
    def __init__(self, **kwargs):
        """ Args: kwargs: See Widget.__init__() """

class TableItem(Widget):
    """item widget for the TableRow."""
    @decorate_constructor_parameter_types([str])
    def __init__(self, text='', **kwargs):
        """ Args: text (str); kwargs: See Widget.__init__() """

class TableTitle(Widget):
    """title widget for the table."""
    @decorate_constructor_parameter_types([str])
    def __init__(self, title='', **kwargs):
        """ Args: title (str) ; kwargs: See Widget.__init__() """

Tables/Grids are all-important widgets in many applications, so more functionality will be needed.

First, a side note about the from_2d_matrix method: it works well, but I found it a bit confusing at first. By analogy with the new_from_list methods for ListView and DropBox, which I'd used first, I sort of assumed it was also a constructor (although it doesn't have new in its name), which of course it isn't :-) Also, the "Fills the table" docstring can be misleading: it actually appends rows to whatever is already in the table. Maybe it should be renamed to append_from_2d_matrix or something ?... It would probably come in handy to have a convenience function like append_row, which would take a list, wrap it into and array and simply call from_2d_matrix with fill_title=False ?

Next, classic Table functionality like keyword args in the constructor for styling Title, odd and even rows.

An empty method like in ListView/DropDown, perhaps with an option to keep the title ?

And, if possible, an event listener/handler to report the row/col being selected...

That's all for now, but I'm sure I can think of more :-)

dddomodossola commented 7 years ago

@fpp-gh I think you are right about the comparison between from_2d_matrixand new_from_list. I would like to rename to new_from_2d_matrix.

The empty method is already implemented because part of the Widget superclass. We can override this method accepting the keep_title boolean parameter as you suggested. Maybe in a DB based app it could be usefull when you reload different table parts.

About event listener/handler. Is it really useful to have row/column of the selected cell? I think it could be better to have the clicked TableItem instance. Doesn't it?

nzjrs commented 7 years ago

I think new_from_list is a better name. Just document that it accepts a list of tuples

On 31 Oct 2016 19:12, "Davide Rosa" notifications@github.com wrote:

@fpp-gh https://github.com/fpp-gh I think you are right about the comparison between from_2d_matrixand new_from_list. I would like to rename to new_from_2d_matrix.

The empty method is already implemented because part of the Widget superclass. We can override this method accepting the keep_title boolean parameter as you suggested. Maybe in a DB based app it could be usefull when you reload different table parts.

About event listener/handler. Is it really useful to have row/column of the selected cell? I think it could be better to have the clicked TableItem instance. Doesn't it?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/dddomodossola/remi/issues/154#issuecomment-257373854, or mute the thread https://github.com/notifications/unsubscribe-auth/AACjoyoZ3cpYDhHRTlzmOPPsrUqp63LTks5q5i9ygaJpZM4KkKxV .

dddomodossola commented 7 years ago

You can see a preview of the changes in development branch.

fpp-gh commented 7 years ago

Yeah, I've had a look, and I like the new methods a lot ! More logical and easier to use, for the most frequent table usage.

As for the events handler(s), I can't tell you what solution is best, but I can tell you about my usual use cases, which I guess are very commonplace:

Let's say I just filled a Table with plenty of rows from a file. Now I need a way to interact with the table to allow row-specific actions (delete, clone, edit...), by way of a popup menu, dialog, etc. But if the entire table is "touchable" it causes problems while scrolling on touch UIs like Android phones. In the Kivy version I had solved that by making the first cell of each row a Button that displayed the cell value and also showed a context menu when pressed. It was ugly and kludgy but it worked because the Kivy Grid allows any kind of object in the cells.

Right now I have ported most of that app to REMI, in much cleaner and concise code. The main missing functionality is this table interaction part. So, one way or another, the handler should provide a way to determine the pressed column, so that the event can be ignored if it not part of the "active" area, and the row data.

And a "delete_row" method (by number or object or whatever) might come in handy, too :-)

Last, let's not forget the title and rows styling :-)

fpp-gh commented 7 years ago

re: event listener/handler:

I recently realized that my ideas above for Table interaction APIs are probably wrong, a legacy of the ugly hacks I did in Kivy, for lack of understanding of its Grid object model.

The Table model is simpler, and should stay simple.

I haven't had time to code and test it yet, but I suspect most needs (well at least mine :-) could be met with two additions to the API (a mix of yours and mine actually :-):

  1. add row and col attributes to TableItem, and set them in the table-filling methods
  2. add an event listener that returns both TableItem and TableRow

So:

For example:

Sounds better or not ? :-)

BTW, I have mixed feelings about the TableTitle class : the difference with TableItem is exactly one char :-) You could remove it and still save two lines in from_2d_matrix and others:

    def from_2d_matrix(self, _matrix, fill_title=True):
        """
        Fills the table with the data contained in the provided 2d _matrix
        The first row of the matrix is set as table title
        """
        for child_keys in list(self.children):
            self.remove_child(self.children[child_keys])
        first_row = True
        for row in _matrix:
            tr = TableRow()
            for item in row:
                ti = TableItem(item)
                if first_row and fill_title:
                    ti.type = 'th'
            self.append(tr)
            first_row = False
dddomodossola commented 7 years ago

@fpp-gh I second you about the new table events consideration. I will provide a first implementation about this. I would not remove the TableTitle because of a possible future use. Maybe it will differ for other things.

fpp-gh commented 7 years ago

Great, thanks ! PS: some people may need row titles, too :-)

dddomodossola commented 7 years ago

Give it a look. The changes are in a dedicated branch _tableapi. The widgets_overview_app.py example shows the new api.

fpp-gh commented 7 years ago

Bit busy right now, will give it a spin tonight, thanks !

fpp-gh commented 7 years ago

Okay, found some time to hack on that _tableapi branch (had to fork my own script too, because it's still on the old API, this is getting complicated :-). What's there is fine, right in the spirit above, and works well. I'd say that we're a bit more than halfway there, because: no row/col indexes anywhere :-)

I think my real problem is that, despite reading the code till my head hurts, I still don't understand what's inside a TableRow. Not being iterable doesn't help :-) And I guess the data structure is buried deep down in the bowels of Tag and Widget through inheritance.

Consequently, although I have a working event handler that gives me the source tableitem and tablerow, I don't know how to use the tablerow, either to get all the items in the row, or to calculate which column the tableitem was in (the demo example only shows the easy part :-). So that's two of my three use cases above , dead in the water :) I'm sure you'll show it is possible ; but I still think it would be more user-friendly to pass a list of strings instead of the tablerow, and have the tableitem provide the row/col indexes as attributes. Else I'll bet that other new users will scratch their heads :-)

Two things that tripped me in the current API:

fpp-gh commented 7 years ago

Forgot to mention these: In this ""fork"" I tested two more things I hadn't got around to yet, the refresh of a tab caption and the global theming with the css file. Both work great :-)

fpp-gh commented 7 years ago

Actually I did sort of figure it out myself in the end :-) What I wanted to do in the handler would be like this:

    def table_handler(self, table, row, item):
        itemtext = item.get_text()
        col = [ ti.get_text() for ti in row.children.values()].index(itemtext)
        print col

... but it doesn't work because row.children is dict-like, not list, so the order is wrong. And there could be repeated values in a row anyway. Back to the drawing board :-)

nzjrs commented 7 years ago

Is .children meant to be public? I'm not sure. In any case, it could be replaced with an ordered dictionary, which would let you keep the order at least.

I'm not sure what of this is a good idea. Tree views and tables are amongst the most complicated widgets in most gui toolkits, we should not just keeping throwing api at remi without thinking first about what the minimal useful table api might look like

On 4 Nov 2016 17:56, "fpp" notifications@github.com wrote:

Actually I did sort of figure it out myself in the end :-) What I wanted to do in the handler would be like this:

def table_handler(self, table, row, item):
    itemtext = item.get_text()
    col = [ ti.get_text() for ti in row.children.values()].index(itemtext)
    print col

... but it doesn't work because row.children is dict-like, not list, so the order is wrong. And there could be repeated values in a row anyway. Back to the drawing board :-)

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/dddomodossola/remi/issues/154#issuecomment-258487456, or mute the thread https://github.com/notifications/unsubscribe-auth/AACjo1wy30zodKr49vWiWQgbQvi6w0aXks5q62O4gaJpZM4KkKxV .

fpp-gh commented 7 years ago

No, I didn't mean to say it's a good idea, it certainly isn't :-) I was just trying to see if I could find the col value for the tableitem if the API doesn't provide it: I failed, and in any case users shouldn't have to do it. That's why I'm pushing for col/row attributes at the tableitem level.

As to how the minimal useful table api might look like, I tried my hand at that in the fifth post of this issue. If people think otherwise that's fine, they just need to chime in :-)

nzjrs commented 7 years ago

Col/row attributes at the table item level level prohibit a future strict model/view split (think a javascript client side sorting mechanism). Other gui apis are very careful at this junction.

On 4 Nov 2016 18:30, "fpp" notifications@github.com wrote:

No, I didn't mean to say it's a good idea, it certainly isn't :-) I was just trying to see if I could find the col value for the tableitem if the API doesn't provide it: I failed, and in any case users shouldn't have to do it. That's why I'm pushing for col/row attributes at the tableitem level.

As to how the minimal useful table api might look like, I tried my hand at that in the fifth post of this issue. If people think otherwise that's fine, they just need to chime in :-)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/dddomodossola/remi/issues/154#issuecomment-258496546, or mute the thread https://github.com/notifications/unsubscribe-auth/AACjo3yTHsw9X9GjWq96k7WCFeNckkAkks5q62uwgaJpZM4KkKxV .

fpp-gh commented 7 years ago

Sorry, I don't even understand what you mean :-) I do understand that without this information I can't use Table for even very basic needs. And I don't recall ever seeing a python GUI framework that deliberately doesn't let you have it...

nzjrs commented 7 years ago

I hope every api decision is deliberate.

dddomodossola commented 7 years ago

I understand the point of view of both of you. But, here we have a use case that have to be solved and John I'm sure you can help us solve this elegantly. The developer ( @fpp in this particular case ) needs to know the row/column of a clicked TableItem. The current implementation of Table lacks this information. Can it be derivated? Which kind of API we can propose to fit the need? I remember that Qt offers a simple implementation for the table and a modelView capable. Maybe we can have both.

fpp-gh commented 7 years ago

@nzjrs: as for me, I certify that I have strictly no religion in these matters... I don't design frameworks, I have enough trouble using them to make apps :-)

I'm a user, and I really want to use REMI, and for the most part I've succeeded, with Davide's support. But in this case I just don't see how I could achieve some extremely common tasks, which are part of the old Kivy app I'm porting (and the one before that, etc.).

If someone, anyone, tells me "use this data structure, this object and this method", and it works, I will do that and not care for a second how exactly it is implemented, if it's future-safe (hah) or whatever.

A silly example, if you will: let's say we are implementing a very basic invoice system. The table looks like this:

Creation Date Customer Amount Tax Due Date Vendor
2016/11/05 fpp 100.00 18.6 2016/12/25 dddomodossola

There is an error in that row (maybe the vendor name :-) so I click on it to edit it in a dialog (probably the same as the original entry form). How do I put the fields in their proper place if all I have is a TableRow with items in no particular order ?

Now I have a version of this app that works on touch devices (for free, thanks to REMI :-). Scrolling through a multi-screen table becomes difficult, so I want the touch event to "fire" the edit dialog (or context menu or popup or whatever) only if it happens in a specific, out-of-the-way area (like the first column for example). How do I do this ?

If there is a way, enlighten me. If there is not, the API is lacking :-)