RMG futurization: Python 3 compatibility and PEP-8 compliance #1724
Conversation
Change Travis build language to minimal Remove the sudo setting which is deprecated Move default test stage out of jobs section
Copy of environment_linux.yml with a few changes Removed packages whose Py3 replacements are still in progress Removed version requirement on pydot
Note that the build matrix will be expanded for all test stages Since this is not the master or stable branch, it won't matter However, this will need to be updated before merging into master
For compatibility with Python 2/3
Our implementation of model reduction is slow to the point of being infeasible for any moderately sized mechanism which requries reduction. It is also tightly integrated with parallel processing via scoop, which is also deprecated and no longer works properly.
Parallelization using scoop has been deprecated and replaced by multiprocessing. This removes all of the remaining code. rmgpy/rmg/parreactTest.py and rmgpy/thermo/thermoengineTest.py were removed because they don't contain any tests that run.
Add list() for iterators Standardized imports in Arkane (and import numpy as np) Replaced 'xrange' with 'range' Replaced itervalues() and iteritems() with values() and items() Replace deprecated 'warn' with 'warning' Standardized module docstrings in Arkane Use CamelCase for class names Make classes in Arkane inherit from object Execute a file correctly in exec()
Also replace instances of catching broad exceptions
Only covers the following files: chemkinTest.py constants*.py constraints*.py display.py exceptions.py quantityTest.py rmgObjectTest.py
Before we were getting it from a dependency of a dependency
Previous wildcard imports masked this issue. Thanks to @mliu49 for catching this and making this fix.
Upgrade Modules to Python 3
Due to changes in gprof2dot API
PEP-8 renaming
Inform users if they are using an unsupported Python version
Codecov Report
@@ Coverage Diff @@
## master #1724 +/- ##
==========================================
- Coverage 41.66% 32.65% -9.01%
==========================================
Files 176 87 -89
Lines 30215 26158 -4057
Branches 6256 6874 +618
==========================================
- Hits 12588 8541 -4047
+ Misses 16703 16645 -58
- Partials 924 972 +48
Continue to review full report at Codecov.
|
|
I added a commit implementing a check for the Python version to make it obvious to users if they need to update their environment. Otherwise, I think compiling or running RMG would lead to a non-obvious SyntaxError. Could someone take a look? |
|
Closes #785 🎉 |
| @@ -4,9 +4,9 @@ | |||
| # | |||
| ################################################################################ | |||
|
|
|||
| .PHONY : all minimal main solver check cantherm clean install decython documentation mopac_travis | |||
| .PHONY : all minimal main solver check pycheck arkane clean install decython documentation mopac_travis | |||
There was a problem hiding this comment.
(Outside the scope of this PR) Are our .PHONY targets up to date? Should things like test and eg0 be in there?
There was a problem hiding this comment.
I was also wondering about this. I looked it up, and it seems like phony targets are most important if you have files with the same name as the target. In that case, putting the target in the phony list tells make to execute the target rather than compile the file.
There was a problem hiding this comment.
I have looked over the last commit @mliu49 added and it looks good to me.
Motivation or Problem
Due to the imminent end of Python 2 support, it is important that RMG is updated to be compatible with Python 3.
Description of Changes
This PR is the culmination of almost six week's work by me, @alongd, @amarkpayne, @mjohnson541, and @xiaoruiDong in futurizing RMG-Py and Arkane.
Main outcomes:
This PR also includes the removal of all remaining scoop functionality, given it has been non-functional since parallel processing has been refactored to use Python multiprocessing, and the reduction module, which was heavily tied into the scoop framework and therefore also non-functional.
Merging
The standard test suite and RMG-tests should test most of the key functionality. Since the individual components of this branch have been previously reviewed, I plan on merging this directly once tests pass.
Next Steps
More detailed testing is definitely needed, but my proposed plan is to do that after merging this into master. The main motivation behind that is so we can switch back to the standard contributor workflow as soon as possible, and allow everyone to begin updating existing PRs and more easily make new PRs for fixing any remaining bugs.