Python 3 #160
Python 3 #160
Conversation
* remove plt.close
* update default setting for SI
* return fig in fep.plot
* remove matplotlib.use('agg')
* remove plt.close
* update default setting for SI
* return fig in fep.plot
* remove matplotlib.use('agg')
…nfig.py string formatting
|
Just realized there was a small error in formatting a string in conifg, fixed it in a commit. |
|
With the fix to GromacsWrapper only 10 tests of 73 are failing, Solvate wet octanol, and the two analysis tests |
|
With the fix to GromacsWrapper only 10 tests of 73 are failing, solvation of wet octanol, and the two analysis tests |
| uses: codecov/codecov-action@v1 | ||
| with: | ||
| file: ./coverage.xml | ||
| fail_ci_if_error: true |
There was a problem hiding this comment.
Started CI file similar to one by @orbeckst for gromacswrapper, work in progress.
mdpow/equil.py
Outdated
|
|
||
| super(Simulation, self).__init__(**kwargs) | ||
|
|
||
| @staticmethod | ||
| def load_pickle(): |
There was a problem hiding this comment.
For compatibility between 2 and 3, could probably also use six
There was a problem hiding this comment.
six is preferred — better use existing, tested libraries and six is standard and already used
| yield has_identifier, identifier | ||
| yield itp_in_top, identifier | ||
| yield coordinates_in_top, identifier | ||
| def test_watermodelsdat(self): |
There was a problem hiding this comment.
Adding parameterized tests to replace yield tests
| @@ -1,4 +1,4 @@ | |||
| path: mdpow/tests/testing_resources/states | |||
| path: /nfs/homes3/alescoul/MDPOW_dev/mdpow/tests/testing_resources/states | |||
There was a problem hiding this comment.
Was having issues finding path, will need to replace.
| for k,v in self.iteritems(): | ||
| x[k] = copy.deepcopy(v, memo) | ||
| return x | ||
| if sys.version_info.major == 3: |
There was a problem hiding this comment.
iteritems() method not available in python 3, Iterable not variable in python 2, would like more elegant solution, but this should function.
mdpow/restart.py
Outdated
| @staticmethod | ||
| def get_pickle(): | ||
| if sys.version_info.major == 3: | ||
| import _pickle as pickle |
There was a problem hiding this comment.
Ugly compatibility code probably could use six
There was a problem hiding this comment.
use six if possible — from six.moves import pickle probably, see docs and/or try it out interactively
|
|
||
| def merge_dicts(user, default): | ||
| # TODO test merge_dicts |
There was a problem hiding this comment.
replaced with recursive method for merging dict containing multiple other dict, passed runinput
There was a problem hiding this comment.
Current draft for update to python 3. Modified several outdated sections expecially around dictionary syntax. Replaced merge_dicts with recursive function with method not dependent on isiter. Many changes just adding spacing to follow PEP8. Tests still need to be modified to run generally, at the moment some paths have been set for locations on workstation for sake of simplicity. Some Errors still occur in the analysis tests some of which appear to be associated with numkit integrate.
Defiantly still a work in progress, hopefully nearing a functioning version.
mdpow/tests/test_Gsolv.py
Outdated
| from mdpow.fep import * | ||
| from mdpow.equil import * |
There was a problem hiding this comment.
avoid blanket imports — only import what you need or better, use the qualified names as before
mdpow/tests/test_analysis.py
Outdated
| from numpy.testing import assert_array_almost_equal | ||
|
|
||
| from six.moves import cPickle as pickle | ||
|
|
||
| import mdpow.fep |
There was a problem hiding this comment.
leave as before with full paths as it makes it MUCH clearer where code comes from
There was a problem hiding this comment.
I'll make that replacement in fep and equil in a new commit.
| @@ -4,17 +4,18 @@ | |||
|
|
|||
| import yaml | |||
| import pybol | |||
|
|
|||
| import gromacs | |||
| from numpy.testing import assert_array_almost_equal | |||
|
|
|||
| from six.moves import cPickle as pickle | |||
mdpow/tests/test_emin.py
Outdated
| class TestEnergyMinimization(object): | ||
|
|
||
| def setup(self): | ||
| gromacs.config.set_gmxrc_environment('~/.conda/envs/MDPOW3/bin/GMXRC') |
There was a problem hiding this comment.
I'll try without it, I was having some issues with it not finding the conda GMXRC, mostly a temporary solution to get tests running so I could evaluate my code. I defiantly need to remove some of the modifications I made while trying to troubleshoot import errors.
mdpow/tests/test_fep.py
Outdated
| from mdpow.config import * | ||
| from mdpow.fep import * |
mdpow/tests/test_fep.py
Outdated
| @@ -50,7 +50,8 @@ class TestFEPschedule(object): | |||
|
|
|||
| def setup(self): | |||
| # load default bundled configuration | |||
| self.cfg = mdpow.config.get_configuration() | |||
| gromacs.config.set_gmxrc_environment('~/.conda/envs/MDPOW3/bin/GMXRC') | |||
mdpow/tests/test_fep_script.py
Outdated
| class TestFEPScript(object): | ||
| def setup(self): | ||
| gromacs.config.set_gmxrc_environment('~/.conda/envs/MDPOW3/bin/GMXRC') |
|
@orbeckst I looked at your comments and to address the concerns will add six.moves to the files currently using cPickle or _pickle. I'll also go back and try to reverse some of the changes I made while trying to troubleshoot getting tests to run for me. They were a bit finicky to get working in particular surrounding sourcing GMXRC and importing the modules. |
|
Just pushed 3 commits addressing @orbeckst comments on the code. Fixed the test imports and switched to six.moves, also removed the set_gmxrc_environment worked without it. |
|
Close this PR because it is superseded by PR #166. |
Replaced yield tests. Removed type annotations. Added more compatibility code for python 2.