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

Python2 Strings to Python3 Strings #1608

Merged
merged 11 commits into from
May 26, 2017
Merged

Python2 Strings to Python3 Strings #1608

merged 11 commits into from
May 26, 2017

Conversation

erichlf
Copy link

@erichlf erichlf commented May 25, 2017

This PR changes all python2 style strings to python3 style strings. It does not change the scripts_regression_tests.py script nor the code_checker script.

Test suite: code_checker and scripts_regression_tests.py
Test baseline:
Test namelist changes:
Test status: bit for bit

Fixes #1492

User interface changes?:

Code review: @jgfouca @jedwards4b

@jgfouca jgfouca requested a review from jedwards4b May 25, 2017 19:58
Copy link
Contributor

@jedwards4b jedwards4b left a comment

Choose a reason for hiding this comment

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

Please change the name of the PR to reflect it's purpose.

@erichlf erichlf changed the title Erichlf/3strings Python2 Strings to Python3 Strings May 26, 2017
@jedwards4b
Copy link
Contributor

I have rebased to master to resolve conflicts and ran scripts_regression_tests.py on cheyenne.

@erichlf
Copy link
Author

erichlf commented May 26, 2017

@jedwards4b hmmm, so did I and it looks like it took your changes instead of mine. There was a place in wait_for_tests.py which had a repeated format, so I changed it in favor of using the indexed format. I wonder what happened to my push?

@erichlf
Copy link
Author

erichlf commented May 26, 2017

Yep, now I can't pull.

@jedwards4b
Copy link
Contributor

After a rebase you need to reset your local to the remote
git reset --hard
please correct my wait_for_tests.py merge if I did it wrong...

Erich L Foster and others added 11 commits May 26, 2017 10:43
Made all initial string conversions and print conversion, however no
testing has been done.
In the last commit I accidentally over wrote some work before I commited.
This is still untested.
I have done a bunch of fixes after an initial test. There are still
plenty more errors, but I wanted to handle a handful at a time.
After running pylint a few times I have addressed most of the glaring
issues. Now it is time to run a full test suite.
All files now pass the pylint test. Onto scripts_regression_tests.py.
When I was converting strings I broke the test scripts. I have done a
checkout of the original scripts to make sure things are working
correctly. However, other things broke. I will probably start from the
beginning and try small incremental changes first.
Had to do a massive bisection to find which files were causing failures.
After extensive work I have corrected all issues and all tests pass.
Had to do a massive bisection to find which files were causing failures.
After extensive work I have corrected all issues and all tests pass.
Changed it so that instead of repeating an item more than once in format.
we repeat the item in the string, e.g. {0} {0}...
@erichlf
Copy link
Author

erichlf commented May 26, 2017

Okay, hurry things are not conflicting. lol

@mvertens mvertens mentioned this pull request Jun 4, 2017
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.

3 participants