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

Transition from Python 2 to 3 #43

Merged
merged 46 commits into from
Oct 22, 2019
Merged

Transition from Python 2 to 3 #43

merged 46 commits into from
Oct 22, 2019

Conversation

nateharms
Copy link

@nateharms nateharms commented Sep 26, 2019

This PR address recent changes in RMG to move from Python 2 to 3. This transition was needed for a while, but RMG (one of the dependencies) needed to be moved to Python 3 before we were able to make this transition ourself.

Summary of changes:

  • all camelCase variables and methods were transitioned to snake_case formatting (with the exception of Arkane input files and jobs)
  • database files were updated to transition camelCase to snake_case
  • The setting of variables for TS and Conformer objects was refactored for python 3

ToDo:

  • update the environment.yml file to use the new RMG 3 (when the binary gets released)
  • add krsna's openbabel to the environment file
  • assert that TravisCI is able to pass the tests when the RMG 3 binary is released

"",
"externalSymmetry = {}".format(external_symmetry),
"",
"spinMultiplicity = {}".format(conformer.rmg_molecule.multiplicity),
"",
"opticalIsomers = 1",
"optical_isomers = 1",
Copy link
Member

Choose a reason for hiding this comment

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

I think this is part of arkane input file and should remain camelCase

@@ -402,7 +402,7 @@ def write_ts_input(self, transitionstate):
"spinMultiplicity = {}".format(
transitionstate.rmg_molecule.multiplicity),
"",
"opticalIsomers = 1",
"optical_isomers = 1",
Copy link
Member

Choose a reason for hiding this comment

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

Arkane input should be camelCase? (I won't comment on any others - do a full search)

resultFile.write('method = "{0!s}"\n'.format(self.method))
resultFile.write("qmData = {0!r}\n".format(self.qmdata))
with open(file_path, 'w') as results_file:
results_file.write('rxn_label = "{0!s}"\n'.format(self.reaction.label))
Copy link
Member

Choose a reason for hiding this comment

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

If we've changed the syntax of our TS results file, do we have a "load" method that can read both old and new syntaxes? or plans to convert all your old data?

Copy link
Author

Choose a reason for hiding this comment

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

We should probably make it backward compatible, good comment, this will be true for the .kinetics files too

@@ -2,8 +2,8 @@
# encoding: utf-8

name = "intra_H_migration/TS_groups"
shortDesc = u""
longDesc = u"""
short_desc = u""
Copy link
Member

Choose a reason for hiding this comment

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

Did they change the syntax of RMG-database??

Copy link
Author

Choose a reason for hiding this comment

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

They have not, but we changed the syntax of ours. Should we change it back?

@codecov
Copy link

codecov bot commented Oct 22, 2019

Codecov Report

Merging #43 into master will decrease coverage by 1.1%.
The diff coverage is 57.95%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #43      +/-   ##
==========================================
- Coverage   64.32%   63.21%   -1.11%     
==========================================
  Files          27       27              
  Lines        4647     4654       +7     
==========================================
- Hits         2989     2942      -47     
- Misses       1658     1712      +54
Impacted Files Coverage Δ
autotst/data/updateTest.py 54.54% <ø> (ø) ⬆️
autotst/calculator/vibrational_analysis.py 86.3% <100%> (ø) ⬆️
autotst/calculator/orca.py 83.69% <100%> (ø) ⬆️
autotst/conformer/systematicTest.py 96.66% <100%> (ø) ⬆️
autotst/calculator/gaussianTest.py 95.23% <100%> (+0.07%) ⬆️
autotst/job/jobTest.py 98.59% <100%> (+5.56%) ⬆️
autotst/__init__.py 100% <100%> (ø) ⬆️
autotst/calculator/orcaTest.py 97.14% <100%> (ø) ⬆️
autotst/data/baseTest.py 99.01% <100%> (ø) ⬆️
autotst/speciesTest.py 98.75% <100%> (ø) ⬆️
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 02f5f8a...239798d. Read the comment docs.

Nate Harms added 5 commits October 22, 2019 11:39
There is an error when running this on travis, I'm guessing what's happening is that the tearDown is running (removing a directory) while another directory is being created.
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

2 participants