On 9/15/2023 16:15, Doug Smythies wrote:
On 2023.09.15 03:41 Swapnil Sapkal wrote:
In intel_pstate_tracer.py, Gnuplot is used to generate 2D plots.
In current implementation this tracer gives error while importing
the module because Gnuplot is imported from package Gnuplot-py which
does not support python 3.x. Fix this by using pygnuplot package to
import this module.
As described in the prerequisites section, the package name is distribution dependant.
On my distribution the original package name is phython3-gnuplot,
and it is working fine.
sys.version_info(major=3, minor=8, micro=10, releaselevel='final', serial=0)
I don't currently have python3-pygnuplot installed, and so this patch breaks
the intel_pstate_tracer for me.
So, I installed the python3-pygnuplot package, and it still didn't work, as there
still wasn't a pygnuplot module to import.
So, I found something called PyGnuplot.py and so changed to that and got further.
But then it got upset with:
File "./intel_pstate_tracer.py.amd", line 298, in common_gnuplot_settings
g_plot = gnuplot.Gnuplot(persist=1)
NameError: name 'gnuplot' is not defined
I gave up and returned to the unpatched
intel_pstate_tracer.py
And checked that is still worked fine. It did.
So, I do not accept this proposed patch.
Not really related, but for a few years now I have been meaning to
change the minimum python version prerequisite to >= 3.0 and
to change the shebang line from this:
#!/usr/bin/env python
To this:
#!/usr/bin/env python3
I have to use the latter version on my distro.
Back when I looked into it, things were inconsistent,
so I didn't know what to do. The kernel tree has 52 .py files
of the latter shebang and 11 of the former.
... Doug
Presumably this is the one that Swapnil intended:
https://pypi.org/project/py-gnuplot/
It requires python3, so I think if upgrading to this one the script does
need to be switched to python3. Besides the shebang, you should also
use a helper like 2to3 to look for any other changes.
There were 97 hits for 'gnuplot' at pypi. 2 stood out but at least in
the case of gnuplot based stuff, I think it's worth dropping
a comment that links back to pypi page for the intended package.
Another alternative is to include a 'requirements.txt' file that pip can
pick up.
https://pip.pypa.io/en/stable/reference/requirements-file-format/
Signed-off-by: Swapnil Sapkal <swapnil.sapkal@xxxxxxx>
---
tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py | 1 -
tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py | 4 ++--
2 files changed, 2 insertions(+), 3 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 2448bb07973f..14f8d81f91de 100755
--- a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
+++ b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
@@ -27,7 +27,6 @@ import re
import signal
import sys
import getopt
-import Gnuplot
from numpy import *
from decimal import *
sys.path.append(os.path.join(os.path.dirname(__file__), '../intel_pstate_tracer'))
diff --git a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
index ec3323100e1a..68412abdd7d4 100755
--- a/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
+++ b/tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
@@ -32,7 +32,7 @@ import re
import signal
import sys
import getopt
-import Gnuplot
+from pygnuplot import gnuplot
from numpy import *
from decimal import *
@@ -295,7 +295,7 @@ def common_all_gnuplot_settings(output_png):
def common_gnuplot_settings():
""" common gnuplot settings. """
- g_plot = Gnuplot.Gnuplot(persist=1)
+ g_plot = gnuplot.Gnuplot(persist=1)
# The following line is for rigor only. It seems to be assumed for .csv files
g_plot('set datafile separator \",\"')
g_plot('set ytics nomirror')
--
2.34.1