Re: [Fuego] [PATCH] Convert the print statement to the print() function

Sireesha,

I like the effort to make the scripts compatible with both the python2
and python3 interpreters, but I have some questions about portions of the p=
atches.


> -----Original Message-----
> From: [email protected] <[email protected]=
m>
>=20
> From: sireesha <[email protected]>
>=20
> Update parentheses for all print statements in python script.
> It is compatible with both python2 and python3 interpreters.
>=20
> Signed-off-by: sireesha <[email protected]>
> ---
>  tests/Benchmark.IOzone/parser.py   |  4 ++--
>  tests/Benchmark.Stream/parser.py   |  4 ++--
>  tests/Benchmark.bonnie/parser.py   |  8 ++++----
>  tests/Benchmark.lmbench2/parser.py | 13 ++++++-------
>  4 files changed, 14 insertions(+), 15 deletions(-)
>  mode change 100755 =3D> 100644 tests/Benchmark.lmbench2/parser.py
>=20
> diff --git a/tests/Benchmark.IOzone/parser.py b/tests/Benchmark.IOzone/pa=
rser.py
> index b1631ae..8a7d5b2 100755
> --- a/tests/Benchmark.IOzone/parser.py
> +++ b/tests/Benchmark.IOzone/parser.py
> @@ -1,11 +1,11 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3

In my setup of Fuego, this will have no effect.  All invocations of the par=
ser in Fuego
execute parser.py as an argument of 'python'.  Specifically, see the run_py=
thon
function in fuego-core/scripts/common.sh.  The actual invocation looks like=
 this:

PATH=3D$ORIG_PATH TOOLCHAIN=3D$TOOLCHAIN python "$@"
(where the $@ arguments contain, among other things, 'parser.py')

I believe Toshiba is using Fuego in a configuration where Fuego is run nati=
vely
(outside the docker container).  Does /usr/bin/python in your environment d=
efault
to python3?

Or, did Toshiba change the run_python function to call python3 instead? or =
to directly
executed the parser.py script?

I need to understand this so that I can decide what the best solution is fo=
r
supporting both docker-based and native-based Fuego installations, as well =
as
supporting legacy Fuego docker containers (which might not have a compatibl=
e
python3 interpreter).

Also...
If this change (to the #! header for the parser script) was needed, it woul=
d be needed
for all tests you are running.  Or, at least, I see unparenthesized print s=
tatements
in the following tests:
IOzone, Stream, x11perf, gtkperf, tiobench, LTP_one_test, Interbench, iperf=
,
ebizzy, bonnie, BF_power_test, aim7, blobsallad, fs_mark, nbench_byte, ptes=
t, lmbench2

It looks like the following test already have parenthesized print statement=
s:
arch_time, LTP, autopkgtest, fuego_ftc_test, fuego_tguid_check, fuego_check=
_tables, fio, and dd


I guess my question is what tests are you running (only those 4, or those 4=
 and others)? and Do
you plan to modify all the tests that have unparenthesized print statements=
 (ie, with additional patches)?

If so, I'll hold off doing it myself.  But if you don't plan to look at the=
 others, I'll do a sweep through
the existing tests and adjust all print statements to be compatible with bo=
th python2 and python3.

>=20
>  import os, sys
>  import common as plib
>=20
>  measurements =3D {}
>  cur_file =3D open(plib.TEST_LOG,'r')
> -print "Reading current values from " + plib.TEST_LOG +"\n"
> +print("Reading current values from " + plib.TEST_LOG +"\n")

This print (which a lot of parsers have) is not really that important.  It'=
s more
of a debug statement, and could probably be replaced with a call to dprint(=
)
(which is defined in common.py, which is always imported as plib (!) - ther=
e are
so many oddities in this legacy code!), or removed entirely.

In any event, I guess I should resist the urge to change all these to dprin=
ts().
Doing so would change the test's default output, which might be a
backward-compatibility-breaking change.

Having said all that, this change to parenthesize the print statement is fi=
ne.

>=20
>  lines =3D cur_file.readlines()
>  cur_file.close()
> diff --git a/tests/Benchmark.Stream/parser.py b/tests/Benchmark.Stream/pa=
rser.py
> index a564d74..69edd27 100755
> --- a/tests/Benchmark.Stream/parser.py
> +++ b/tests/Benchmark.Stream/parser.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
Same issue here as above.

>=20
>  import os, re, sys
>  import common as plib
> @@ -7,7 +7,7 @@ ref_section_pat =3D "^\[[\w]+.[gle]{2}\]"
>  # groups           1        2
>  cur_search_str =3D "^(\w*):\ *([\d]{1,5}.[\d]{1,4}).*"
>=20
> -print "Reading current values from " + plib.TEST_LOG
> +print("Reading current values from " + plib.TEST_LOG)
>  cur_file =3D open(plib.TEST_LOG,'r')
>=20
>  lines =3D cur_file.readlines()
> diff --git a/tests/Benchmark.bonnie/parser.py b/tests/Benchmark.bonnie/pa=
rser.py
> index b68bc3e..5f35af4 100755
> --- a/tests/Benchmark.bonnie/parser.py
> +++ b/tests/Benchmark.bonnie/parser.py
> @@ -5,16 +5,16 @@ import common as plib
>=20
>  measurements =3D {}
>=20
> -print "Reading current values from " + plib.TEST_LOG + "\n"
> +print("Reading current values from " + plib.TEST_LOG + "\n")
>  with open(plib.TEST_LOG,'r') as cur_file:
>      raw_values =3D cur_file.readlines()
>      results =3D raw_values[-1].rstrip("\n").split(",")
>=20
>  if len(results) < 26:
> -    print "\nFuego error reason: No results found\n"
> +    print("\nFuego error reason: No results found\n")
>      sys.exit(2)
>=20
> -print "Bonnie++ raw results: " + str(results)
> +print("Bonnie++ raw results: " + str(results))
>=20
>=20
>  measurements["Sequential_Output.PerChr"] =3D []
> @@ -81,6 +81,6 @@ if not '+' in results[26]:
>=20
>  # Add a hint when the spec seems to require a bigger SIZE
>  if '+' in results[4]:
> -    print "\nWARNING: Some test result data is missing. You might need b=
igger test data size.\nTry the test using the 'more-data' spec
> for better results.\n"
> +    print("\nWARNING: Some test result data is missing. You might need b=
igger test data size.\nTry the test using the 'more-data' spec
> for better results.\n")
>=20
>  sys.exit(plib.process(measurements))
> diff --git a/tests/Benchmark.lmbench2/parser.py b/tests/Benchmark.lmbench=
2/parser.py
> old mode 100755
> new mode 100644
> index f06f132..5ccfe97
> --- a/tests/Benchmark.lmbench2/parser.py
> +++ b/tests/Benchmark.lmbench2/parser.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
>=20
>  import os, re, sys
>  import common as plib
> @@ -11,7 +11,7 @@ cur_search_pat =3D re.compile("^\S+\s+Linux\s[0-9a-z.-]=
+\s+([\s\d.KMGT-]*)",re.MUL
>=20
>  cur_dict =3D {}
>  cur_file =3D open(plib.TEST_LOG,'r')
> -print "Reading current values from " + plib.TEST_LOG +"\n"
> +print("Reading current values from " + plib.TEST_LOG +"\n")
>=20
>  lines =3D cur_file.readlines()
>=20
> @@ -19,18 +19,17 @@ lines =3D cur_file.readlines()
>  t_index =3D 0
>  sublist =3D []
>=20
> -print lines
> +print(lines)
>=20
>  for line in lines:
>      result =3D cur_search_pat.findall(line)
>      if result and len(result[0]) > 0:
> -        print 'result: ', result
> +        print('result: ', result)
>=20
>          # Ugly hack to work around invalid processing of empty cells
>          result[0] =3D result[0].replace('       ', '     0 ')
>          test_res =3D result[0].rstrip('\n').split(' ')
> -
> -        print "test_res =3D %s" % (test_res)
> +        print("test_res =3D %s" % (test_res))
>=20
>          if 0 < t_index < 9:
>              for value in test_res:
> @@ -104,6 +103,6 @@ cur_dict["Memory_Latencies.Main_mem"] =3D sublist[48]
>  #cur_dict["Memory_Latencies.Guesses"] =3D sublist[49]
>  cur_dict["Memory_Latencies.Rand_mem"] =3D sublist[50]
>=20
> -print "cur_dict =3D %s" % (cur_dict)
> +print("cur_dict =3D %s" % (cur_dict))
This is also one of those debug statements that should probably be a dprint=
().

I'm more inclined to change this one, because this is just showing internal
data of the parser, which really should not be displayed in the non-debug c=
ase.

>=20
>  sys.exit(plib.process_data(ref_section_pat, cur_dict, 'xl', ' '))
> --
> 2.20.1
>=20

OK.  Overall, all the print statement conversions are fine.  However, I don=
't want to change
the #!/usr/bin/python lines, as I don't think that has any actual effect.  =
If it does need to be
changed, it will need to be changed globally in all parser.py files.  AND I=
'll have to go back and
test to make sure that this will work with previous docker containers (from=
 Fuego 1.2 and above) to make
sure they have a compatible python3 interpreter installed.

Thanks for raising this issue.  Please let me know the answers to my questi=
ons, and we'll get
this change in (and possibly other related changes).=20

We may have to drag Fuego kicking and screaming into full python3 compatibi=
lity. :-)

 -- Tim