Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating the test script to work with python 3.x #918

Merged
merged 1 commit into from Sep 1, 2020

Conversation

shivaramaarao
Copy link
Collaborator

The script check_compilation.py is written for python 2.x and many tests fail when python 3.x is used.
The print statements used in the script such as print title are not compatible with python 3.x
The script is updated to support both python 2.x and pyton 3.x.

Copy link
Collaborator

@bryanpkc bryanpkc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@@ -101,7 +102,7 @@ def getLogErrors(log):
elif terminatedError.search(line):
terminated = True
else:
print line,
sys.stdout.write (line)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a python expert but wouldn't we lose the space between printed lines if we directly use sys.stdout.write?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original Python 2 statement print line, with a trailing comma, would have skipped the newline. sys.stdout.write gives the same behavior, and works with both Python 2 and 3.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newline is fine and it works with both 2 and 3. The question is about space.

I was trying the python program below and with python 2 it gave the following output. Where the print with comma added space but sys.stdout.write does not add a space.
one two three
onetwothree

import sys
mylist = ["one", "two", "three"]
for l in mylist:
    print l,
print ""
for l in mylist:
    sys.stdout.write(l)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. this may be a problem and print (line, endl=" ") is the correct code for python 3.x.
Looks like I need to add additional sys.stdout.write(" ") line. would that be fine?

Copy link
Collaborator

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@gklimowicz gklimowicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM too.

@sscalpone
Copy link
Member

Who tested this patch with python2 and python3?

@shivaramaarao
Copy link
Collaborator Author

Who tested this patch with python2 and python3?

Hi Steve, I tested with python 2.7 and python 3.6. do you think some more testing need to be done?

@sscalpone
Copy link
Member

@shivaramaarao Hi, that's great. Thanks

@kiranchandramohan
Copy link
Collaborator

I tested on python2 and on python3 (by changing the invocation to use python3 and also setting python3 as alias for python).

openmp_examples/makefile:COMP_CHECK = python3 $(BASE_DIR)/../tools/check_compilation.py
f90_correct/makefile:COMP_CHECK=python3 $(HOMEQA)/../tools/check_compilation.py

@shivaramaarao shivaramaarao merged commit da0665e into master Sep 1, 2020
@gklimowicz
Copy link
Contributor

I also tested on Power with Python2 and Python3 by changing the scripts that set up the location of the Python executable.

@shivaramaarao shivaramaarao deleted the shivaram_fixes branch April 23, 2023 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants