ListView type hints

[fkaab]

Added type hints for the type of ListItem contained in a ListView.

• Docstrings on all new or modified functions / classes
• Updated documentation
• Updated CHANGELOG.md (where appropriate)

[rodrigogiraoserrao]

Hi @fkaab, thanks for your PR.
I don't understand what you are trying to do, though.
You are making changes to typing but if I run make typecheck, it seems that your changes introduce more type errors in the code base.

What is the problem that your PR solves?
What are you trying to achieve?

If you hit a problem while using Textual, it is always a good idea to start by opening an issue so that everyone is made aware of the problem you are facing and so that we can discuss possible solutions.

Thanks!

[fkaab]

Hello @rodrigogiraoserrao,
I want to add type hints to ListView such that I can hint at the type of the ListItem contained in a ListView. I'd like to be able to hint at these like

my_list: ListView[MyListItemClass] = ListView()

or similarly subclass ListView

class MyListViewClass(ListView[MyListItemClass]):
    ...

As of now this is highlighted as erroneous in my IDE. The PR changes that. Also I have now fixed most of the issues highlighted by the typecheck.

[davep]

@rodrigogiraoserrao If it helps, check the type hints and completions in your editor, when this PR is your active Textual:

from textual import on
from textual.app import App, ComposeResult
from textual.widgets import Button, ListView, ListItem, ProgressBar

class ProgressItem(ListItem):

    def compose(self) -> ComposeResult:
        yield ProgressBar()

class FunList(ListView[ProgressItem]):
    pass

class VariedLVApp(App[None]):

    def compose(self) -> ComposeResult:
        yield Button("More fun")
        yield FunList(
            ProgressItem(),
            ProgressItem(),
            ProgressItem(),
            ProgressItem(),
            ProgressItem(),
        )

    @on(Button.Pressed)
    def more_fun(self) -> None:
        self.query_one(FunList).append(ProgressItem())

if __name__ == "__main__":
    VariedLVApp().run()

I did have a look into this once before, albeit more concentrating on typing ListItem (#1603) but ran into a couple of issues. On the surface this PR seems like a more sensible and pragmatic approach.

I did have a concern about how useful and viable it was when it came to message handlers with multiple ListItems of differing types, @fkaab perhaps you could extend your description a little bit to take this into account and note any issues and corner cases there could be there?

Since then we've also added @on, so I imagine some thought might also need to be given to that too.

[fkaab]

I have played around with your example for a bit @davep, and everything seems to work fine. There are three things I would like to note:

• I suggest adding a TypeVar for ListItem to its definition for the type hint of the item of ListItem._ChildClicked to make the type inference for the messages emitted from ListItem subclasses a little bit nicer. I tried it and it didn't create any issues with the type check. If you agree I would update the PR.
• The only edge cases I can think of are nested ListViews, i.e. ListItems that contain ListView. When a ListItem._ChildClicked is emitted both the outer and inner ListView would react to it. If we stopped the message in ListView.on_list_item__child_clicked we could type hint that the item of messages emitted by ListView are of the parametric ListItem subclass type contained in the ListView. Since Click is propagated anyway, ListView.Highlighted would also be emitted and _ChildClicked is a private attribute, I think it is a small and reasonable breaking change to stop _ChildClick in ListViews unless it goes against some previous policy. If you agree I would update the PR.
• The last remaining typecheck error related to this change stems from the lack of generic types in NodeList. One could remedy this, but I don't know if you would want to add generic types to all internal classes, and if that might create any issues down the line. I won't have time to do this for at least five or six months however.

On an unrelated note: I am not really a professional software developer and was kind of unaware of the make tool used in this project and its capabilities. I suggest adding a line to the contribution guidelines about running the typecheck when one introduces new type hints or new classes.

[davep]

Thanks @fkaab. I think I'm struggling to follow/envisage what you're referring to in your first point, do you think you could further illustrate it for me in some way?

The second point, I think, sounds like a more general bug in ListView, right? (honestly I very much doubt ListView was created with the idea of a ListView in ListItem being a thing anyone would do). However, if that can be cleaned up with no negative impact, that seems like a good idea.

As for the third point: yeah, that seems very out of scope for what's going on in this PR.

Good point on helping people discover the make commands; when you get a moment would you mind perhaps posting a request for this to the "Ideas" discussion area? Anything that makes working on Textual more accessible to a wider audience sounds like a good thing to me.

[willmcgugan]

@davep @rodrigogiraoserrao This seems reasonable to me. What do you guys think?

[davep]

@willmcgugan I think I remember it looking like a good approach, but it's like 5 months since.

[rodrigogiraoserrao]

@willmcgugan I think I remember it looking like a good approach, but it's like 5 months since.

Ditto.