Re: [PATCH] tools/power: amd_pstate_trace: fix help path dependencies

Hi Prateek, Mario,

Agreed. Moving the NumPy import below argument parsing is not the
right way to manage a required dependency. Please drop this version.

I will keep the NumPy import at module scope, split out the narrow -h
output fix, and handle dependency/venv documentation separately after
verifying the shared requirements for both tracers. The unused Gnuplot
import in amd_pstate_trace can still be removed as a separate cleanup.

Thanks for the guidance.

Regards,
Yousef

On Wed, 22 Jul 2026 10:32:41 -0500, Mario Limonciello
<[email protected]> wrote:
> On 7/22/26 10:21, K Prateek Nayak wrote:
> > Hello Rafael,
> >
> > On 7/22/2026 6:18 PM, Rafael J. Wysocki (Intel) wrote:
> >> On Wed, Jun 24, 2026 at 2:31 PM Yousef Alhouseen
> >> <[email protected]> wrote:
> >>>
> >>> amd_pstate_trace imports Gnuplot and numpy before parsing command-line
> >>> options. As a result, even "-h" fails if those optional runtime modules are
> >>> not installed.
> >>>
> >>> It also handles "-h" by printing a blank line and exiting. Use the existing
> >>> help printer for "-h", and move the numpy import to the path that needs it.
> >>> The direct Gnuplot import is unused, so remove it.
> >>>
> >>> Signed-off-by: Yousef Alhouseen <[email protected]>
> >>
> >> AMD folks, if this change is fine with you, I can take it into
> >> pm-tools, but an ACK (or equivalent) is needed.
> >
> > Sorry for the delay in reviewing! I have some comments below for Yousef.
>
> Yes; sorry this got buried in my inbox when I was out and I still
> haven't all the way dug out.
>
> >
> >>
> >> Thanks!
> >>
> >>> ---
> >>> tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py | 11 +++++------
> >>> 1 file changed, 5 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
> >>> index 875b08655..b1b201b2e 100755
> >>> --- a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
> >>> +++ b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
> >>> @@ -27,8 +27,6 @@ import re
> >>> import signal
> >>> import sys
> >>> import getopt
> >>> -import Gnuplot
> >
> > We don't use Gnuplot so removing that is fine ...
> >
> >>> -from numpy import *
> >
> > ... but we want to keep all the imports together at top to easily audit
> > dependencies. Afacit, most python scripts in tools/power/x86/ follow
> > that convention.
> >
> > There exists a big fat comment on top of the file reading:
> >
> > Prerequisites:
> > Python version 2.7.x or higher
> > gnuplot 5.0 or higher
> > gnuplot-py 1.8 or higher
> >
> > which is a good indication of whatis actually required to run the
> > script. If users prefer, we can add a requirements.txt that pip can
> > consume to install the necessary dependencies and make it clear.
>
> I think a requirements.txt and a quick few steps of how to start a venv
> and use it is a great idea.
>
> >
> >>> from decimal import *
> >>> sys.path.append(os.path.join(os.path.dirname(__file__), "..", "intel_pstate_tracer"))
> >>> import intel_pstate_tracer as ipt
> >>> @@ -260,9 +258,6 @@ graph_data_present = False;
> >>> valid1 = False
> >>> valid2 = False
> >>>
> >>> -cpu_mask = zeros((MAX_CPUS,), dtype=int)
> >>> -
> >>> -
> >>> try:
> >>> opts, args = getopt.getopt(sys.argv[1:],"ht:i:c:n:m:",["help","trace_file=","interval=","cpu=","name=","memory="])
> >>> except getopt.GetoptError:
> >>> @@ -270,7 +265,7 @@ except getopt.GetoptError:
> >>> sys.exit(2)
> >>> for opt, arg in opts:
> >>> if opt == '-h':
> >>> - print()
> >>> + ipt.print_help('amd_pstate')
> >>> sys.exit()
> >>> elif opt in ("-t", "--trace_file"):
> >>> valid1 = True
> >>> @@ -291,6 +286,10 @@ if not (valid1 and valid2):
> >>> ipt.print_help('amd_pstate')
> >>> sys.exit()
> >>>
> >>> +from numpy import zeros
> >
> > Having it here needs users to skim through the file to know what
> > external packages the script needs and we don't want that.
> >
> >>> +
> >>> +cpu_mask = zeros((MAX_CPUS,), dtype=int)
> >>> +
> >>> if cpu_list:
> >>> for p in re.split("[,]", cpu_list):
> >>> if int(p) < MAX_CPUS :
> >>> --
> >>> 2.54.0
> >>>
> >>>
> >