All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Namhyung Kim <namhyung@kernel.org>, Jiri Olsa <jolsa@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	James Clark <james.clark@linaro.org>,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] perf python: Fix ilist TreeNode string type drift
Date: Sun, 13 Sep 2026 18:40:14 -0300	[thread overview]
Message-ID: <aqcYPoBOEKNk0WMc@x2> (raw)
In-Reply-To: <20260910211539.3157204-1-irogers@google.com>

On Thu, Sep 10, 2026 at 02:15:39PM -0700, Ian Rogers wrote:
> Updates Tree type hints to correctly use TreeNode[TreeValue]
> across all assignments preventing mypy strictness mismatches.
> 
> Address some other minor stylistic issues reported by mypy, pylint and
> Sashiko.

Thanks, applied to perf-tools-next, for v7.4.

- Arnaldo
 
> Assisted-by: Antigravity:gemini-3.1-pro
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> v2: Tweak the imports and type annotations for better 3.9
>     compatibility and to be more pythonic.
> ---
>  tools/perf/python/ilist.py | 78 ++++++++++++++++++++------------------
>  1 file changed, 42 insertions(+), 36 deletions(-)
> 
> diff --git a/tools/perf/python/ilist.py b/tools/perf/python/ilist.py
> index 0d757ddb4795..ebff0a843b7a 100755
> --- a/tools/perf/python/ilist.py
> +++ b/tools/perf/python/ilist.py
> @@ -2,11 +2,13 @@
>  # SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
>  """Interactive perf list."""
>  
> -from abc import ABC, abstractmethod
> +from __future__ import annotations
> +
>  import argparse
> +from abc import ABC, abstractmethod
>  from dataclasses import dataclass
>  import math
> -from typing import Any, Dict, Optional, Tuple
> +from typing import Any, Optional
>  import perf
>  from textual import on
>  from textual.app import App, ComposeResult
> @@ -19,7 +21,7 @@ from textual.widgets import Button, Footer, Header, Input, Label, Sparkline, Sta
>  from textual.widgets.tree import TreeNode
>  
>  
> -def get_info(info: Dict[str, str], key: str):
> +def get_info(info: dict[str, Any], key: str):
>      return (info[key] + "\n") if key in info else ""
>  
>  
> @@ -140,7 +142,7 @@ class ErrorScreen(ModalScreen[bool]):
>      def compose(self) -> ComposeResult:
>          yield Button(f"Error: {self.error}", variant="primary", id="error")
>  
> -    def on_button_pressed(self, event: Button.Pressed) -> None:
> +    def on_button_pressed(self) -> None:
>          self.dismiss(True)
>  
>  
> @@ -234,7 +236,7 @@ class IListApp(App):
>  
>      def __init__(self, interval: float) -> None:
>          self.interval = interval
> -        self.evlist = None
> +        self.evlist: Optional[perf.evlist] = None
>          self.selected: Optional[TreeValue] = None
>          self.search_results: list[TreeNode[TreeValue]] = []
>          self.cur_search_result: TreeNode[TreeValue] | None = None
> @@ -284,18 +286,18 @@ class IListApp(App):
>              """Sets the focus after the SearchScreen is dismissed."""
>  
>              search_label = self.query_one("#active_search", Label)
> -            search_label.display = True if event else False
> +            search_label.display = bool(event)
>              if not event:
>                  return
>              event = event.lower()
>              search_label.update(f'Searching for events matching "{event}"')
>  
> -            tree: Tree[str] = self.query_one("#root", Tree)
> +            tree: Tree[TreeValue] = self.query_one("#root", Tree)
>  
> -            def find_search_results(event: str, node: TreeNode[str],
> +            def find_search_results(event: str, node: TreeNode[TreeValue],
>                                      cursor_seen: bool = False,
> -                                    match_after_cursor: Optional[TreeNode[str]] = None
> -                                    ) -> Tuple[bool, Optional[TreeNode[str]]]:
> +                                    match_after_cursor: Optional[TreeNode[TreeValue]] = None
> +                                    ) -> tuple[bool, Optional[TreeNode[TreeValue]]]:
>                  """Find nodes that match the search remembering the one after the cursor."""
>                  if not cursor_seen and node == tree.cursor_node:
>                      cursor_seen = True
> @@ -332,7 +334,7 @@ class IListApp(App):
>  
>      def action_collapse(self) -> None:
>          """Collapse the part of the tree currently on."""
> -        tree: Tree[str] = self.query_one("#root", Tree)
> +        tree: Tree[TreeValue] = self.query_one("#root", Tree)
>          node = tree.cursor_node
>          if node and node.parent:
>              node.parent.collapse_all()
> @@ -343,31 +345,34 @@ class IListApp(App):
>          if not self.selected or not self.evlist:
>              return
>  
> -        def update_count(cpu: int, count: int):
> +        def update_count(cpu: int, count: int | float):
>              # Update the raw count display.
> -            counter: Label = self.query(f"#counter_cpu{cpu}" if cpu >= 0 else "#counter_total")
> -            if not counter:
> +            counter_query = self.query(f"#counter_cpu{cpu}" if cpu >= 0 else "#counter_total")
> +            if not counter_query:
>                  return
> -            counter = counter.first(Label)
> +            counter: Label = counter_query.first(Label)
>              counter.update(str(count))
>  
>              # Update the sparkline.
> -            line: Sparkline = self.query(f"#sparkline_cpu{cpu}" if cpu >= 0 else "#sparkline_total")
> -            if not line:
> +            line_query = self.query(f"#sparkline_cpu{cpu}" if cpu >= 0 else "#sparkline_total")
> +            if not line_query:
>                  return
> -            line = line.first(Sparkline)
> +            line: Sparkline = line_query.first(Sparkline)
>              # If there are more events than the width, remove the front event.
> -            if len(line.data) > line.size.width:
> -                line.data.pop(0)
> -            line.data.append(count)
> -            line.mutate_reactive(Sparkline.data)
> +            if line.data is not None:
> +                if len(line.data) > line.size.width:
> +                    line.data = line.data[1:]
> +                line.data = list(line.data) + [float(count)]
> +            else:
> +                line.data = [float(count)]
> +            line.refresh()
>  
>          # Update the total and each CPU counts, assume there's just 1 evsel.
> -        total = 0
> +        total: float = 0.0
>          self.evlist.disable()
>          for evsel in self.evlist:
>              for cpu in evsel.cpus():
> -                aggr = 0
> +                aggr: float = 0.0
>                  for thread in evsel.threads():
>                      aggr += self.selected.value(self.evlist, evsel, cpu, thread)
>                  update_count(cpu, aggr)
> @@ -424,16 +429,16 @@ class IListApp(App):
>          # Add spark lines for all the CPUs. Note, must be done after
>          # open so that the evlist CPUs have been computed by propagate
>          # maps.
> -        line = CounterSparkline(cpu=-1)
> -        lines.mount(line)
> +        line_sp = CounterSparkline(cpu=-1)
> +        lines.mount(line_sp)
>          for cpu in self.evlist.all_cpus():
> -            line = CounterSparkline(cpu)
> -            lines.mount(line)
> -        line = Counter(cpu=-1)
> -        lines.mount(line)
> +            c_sp = CounterSparkline(cpu)
> +            lines.mount(c_sp)
> +        c_val = Counter(cpu=-1)
> +        lines.mount(c_val)
>          for cpu in self.evlist.all_cpus():
> -            line = Counter(cpu)
> -            lines.mount(line)
> +            c_val2 = Counter(cpu)
> +            lines.mount(c_val2)
>  
>      def compose(self) -> ComposeResult:
>          """Draws the app."""
> @@ -459,17 +464,18 @@ class IListApp(App):
>                      # Reading events may fail with EPERM, ignore.
>                      pass
>              metrics = tree.root.add("Metrics")
> -            groups = set()
> +            groups: set[str] = set()
>              for metric in perf.metrics():
>                  groups.update(metric["MetricGroup"])
>  
> -            def add_metrics_to_tree(node: TreeNode[TreeValue], parent: str, pmu: str = None):
> +            def add_metrics_to_tree(node: TreeNode[TreeValue], parent: str, pmu: Optional[str] = None):
>                  for metric in sorted(perf.metrics(), key=lambda x: x["MetricName"]):
> -                    metric_pmu = metric.get('PMU')
> +                    metric_pmu_raw = metric.get('PMU')
> +                    metric_pmu = str(metric_pmu_raw) if metric_pmu_raw else ''
>                      if pmu and metric_pmu and metric_pmu != pmu:
>                          continue
>                      if parent in metric["MetricGroup"]:
> -                        name = metric["MetricName"]
> +                        name = str(metric["MetricName"])
>                          display_name = name
>                          if metric_pmu:
>                              display_name += f" ({metric_pmu})"
> -- 
> 2.55.0.1007.g17ff1f9808-goog

      parent reply	other threads:[~2026-09-13 21:40 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-10 17:44 [PATCH v1] perf python: Fix ilist TreeNode string type drift Ian Rogers
2026-09-10 17:51 ` sashiko-bot
2026-09-12 20:19   ` Arnaldo Carvalho de Melo
2026-09-13  4:33     ` Ian Rogers
2026-09-10 21:15 ` [PATCH v2] " Ian Rogers
2026-09-10 21:18   ` sashiko-bot
2026-09-13 21:40   ` Arnaldo Carvalho de Melo [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aqcYPoBOEKNk0WMc@x2 \
    --to=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.