From ee9b46615b2bcbb1fa3d6757e95c97f97962accd Mon Sep 17 00:00:00 2001 From: Lily Wang Date: Tue, 16 Mar 2021 07:39:31 -0700 Subject: [PATCH 1/8] fix attributeerrors from ts reading --- package/MDAnalysis/coordinates/ParmEd.py | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/package/MDAnalysis/coordinates/ParmEd.py b/package/MDAnalysis/coordinates/ParmEd.py index b2eb92d9c3f..8415f89e29e 100644 --- a/package/MDAnalysis/coordinates/ParmEd.py +++ b/package/MDAnalysis/coordinates/ParmEd.py @@ -211,9 +211,21 @@ def convert(self, obj): atom_kwargs = [] for atom, name, resname, xyz, vel in zip(ag_or_ts, names, resnames, positions, velocities): - akwargs = {'name': name} - chain_seg = {'segid': atom.segid} - for attrname in ('mass', 'charge', 'type', + try: + resid = atom.resid + except AttributeError: + resid = 1 + try: + mass = atom.mass + except AttributeError: + mass = 0 + try: + segid = atom.segid + except AttributeError: + segid = "X" + chain_seg = {'segid': segid} + akwargs = {'name': name, "mass": mass} + for attrname in ('charge', 'type', 'altLoc', 'tempfactor', 'occupancy', 'gbscreen', 'solventradius', 'nbindex', 'rmin', 'epsilon', 'rmin14', @@ -239,7 +251,7 @@ def convert(self, obj): chain_seg['inscode'] = atom.icode except AttributeError: pass - atom_kwargs.append((akwargs, resname, atom.resid, chain_seg, xyz, vel)) + atom_kwargs.append((akwargs, resname, resid, chain_seg, xyz, vel)) struct = pmd.Structure() From 879e4705c25fe04ad46eddbdd4e52919a2edc7d8 Mon Sep 17 00:00:00 2001 From: Lily Wang Date: Tue, 16 Mar 2021 07:40:32 -0700 Subject: [PATCH 2/8] deprecate ts --- package/MDAnalysis/coordinates/ParmEd.py | 4 ++++ testsuite/MDAnalysisTests/coordinates/test_parmed.py | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/package/MDAnalysis/coordinates/ParmEd.py b/package/MDAnalysis/coordinates/ParmEd.py index 8415f89e29e..92c25ea7a74 100644 --- a/package/MDAnalysis/coordinates/ParmEd.py +++ b/package/MDAnalysis/coordinates/ParmEd.py @@ -175,6 +175,10 @@ def convert(self, obj): except AttributeError: if isinstance(obj, base.Timestep): ag_or_ts = obj.copy() + warnings.warn('Passing a Timestep to convert is deprecated, ' + 'and will be removed in 2.0; ' + 'use either an AtomGroup or Universe', + DeprecationWarning) else: raise_from(TypeError("No Timestep found in obj argument"), None) diff --git a/testsuite/MDAnalysisTests/coordinates/test_parmed.py b/testsuite/MDAnalysisTests/coordinates/test_parmed.py index 5dd350e86fd..ab554ad49f2 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_parmed.py +++ b/testsuite/MDAnalysisTests/coordinates/test_parmed.py @@ -31,6 +31,7 @@ from MDAnalysisTests.coordinates.base import _SingleFrameReader from MDAnalysisTests.coordinates.reference import RefAdKSmall +from MDAnalysis.coordinates.ParmEd import ParmEdConverter from MDAnalysisTests.datafiles import ( GRO, @@ -288,3 +289,10 @@ class TestParmEdConverterPDB(BaseTestParmEdConverter): def test_equivalent_coordinates(self, ref, output): assert_almost_equal(ref.coordinates, output.coordinates, decimal=3) + +def test_convert_ts_deprecation(): + u = mda.Universe(PDB_small) + err = "Passing a Timestep to convert is deprecated" + with pytest.warns(DeprecationWarning, match=err): + c = ParmEdConverter() + c.convert(u.trajectory.ts) \ No newline at end of file From 25983904f6e7bbe27453eb1fa2370c8d34a7c694 Mon Sep 17 00:00:00 2001 From: Lily Wang Date: Tue, 16 Mar 2021 07:43:21 -0700 Subject: [PATCH 3/8] pep8 --- package/MDAnalysis/coordinates/ParmEd.py | 8 ++++---- testsuite/MDAnalysisTests/coordinates/test_parmed.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package/MDAnalysis/coordinates/ParmEd.py b/package/MDAnalysis/coordinates/ParmEd.py index 92c25ea7a74..848ef335cb1 100644 --- a/package/MDAnalysis/coordinates/ParmEd.py +++ b/package/MDAnalysis/coordinates/ParmEd.py @@ -175,10 +175,10 @@ def convert(self, obj): except AttributeError: if isinstance(obj, base.Timestep): ag_or_ts = obj.copy() - warnings.warn('Passing a Timestep to convert is deprecated, ' - 'and will be removed in 2.0; ' - 'use either an AtomGroup or Universe', - DeprecationWarning) + warnings.warn('Passing a Timestep to convert is deprecated, ' + 'and will be removed in 2.0; ' + 'use either an AtomGroup or Universe', + DeprecationWarning) else: raise_from(TypeError("No Timestep found in obj argument"), None) diff --git a/testsuite/MDAnalysisTests/coordinates/test_parmed.py b/testsuite/MDAnalysisTests/coordinates/test_parmed.py index ab554ad49f2..b0fa1f43291 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_parmed.py +++ b/testsuite/MDAnalysisTests/coordinates/test_parmed.py @@ -295,4 +295,4 @@ def test_convert_ts_deprecation(): err = "Passing a Timestep to convert is deprecated" with pytest.warns(DeprecationWarning, match=err): c = ParmEdConverter() - c.convert(u.trajectory.ts) \ No newline at end of file + c.convert(u.trajectory.ts) From 60e72685ac337502a9a98176bbbeec22fdc2dbd8 Mon Sep 17 00:00:00 2001 From: Lily Wang Date: Thu, 1 Apr 2021 20:26:24 -0700 Subject: [PATCH 4/8] remove deprecation warning and raise value instead --- package/MDAnalysis/coordinates/ParmEd.py | 9 +++------ testsuite/MDAnalysisTests/coordinates/test_parmed.py | 6 +++--- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/package/MDAnalysis/coordinates/ParmEd.py b/package/MDAnalysis/coordinates/ParmEd.py index 848ef335cb1..823bd2960df 100644 --- a/package/MDAnalysis/coordinates/ParmEd.py +++ b/package/MDAnalysis/coordinates/ParmEd.py @@ -174,13 +174,10 @@ def convert(self, obj): ag_or_ts = obj.atoms except AttributeError: if isinstance(obj, base.Timestep): - ag_or_ts = obj.copy() - warnings.warn('Passing a Timestep to convert is deprecated, ' - 'and will be removed in 2.0; ' - 'use either an AtomGroup or Universe', - DeprecationWarning) + raise ValueError("Writing Timesteps to ParmEd " + "objects is not supported") else: - raise_from(TypeError("No Timestep found in obj argument"), None) + raise_from(TypeError("No atoms found in obj argument"), None) # Check for topology information missing_topology = [] diff --git a/testsuite/MDAnalysisTests/coordinates/test_parmed.py b/testsuite/MDAnalysisTests/coordinates/test_parmed.py index b0fa1f43291..0710186d304 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_parmed.py +++ b/testsuite/MDAnalysisTests/coordinates/test_parmed.py @@ -290,9 +290,9 @@ def test_equivalent_coordinates(self, ref, output): assert_almost_equal(ref.coordinates, output.coordinates, decimal=3) -def test_convert_ts_deprecation(): +def test_pass_ts_error(): u = mda.Universe(PDB_small) - err = "Passing a Timestep to convert is deprecated" - with pytest.warns(DeprecationWarning, match=err): + err = "Writing Timesteps to ParmEd objects is not supported" + with pytest.raises(ValueError, match=err): c = ParmEdConverter() c.convert(u.trajectory.ts) From 35a6938e7b167770c02b732243a2782f98e378e3 Mon Sep 17 00:00:00 2001 From: Lily Wang Date: Sat, 3 Apr 2021 01:26:17 -0700 Subject: [PATCH 5/8] add test and revert overcautious changes --- package/MDAnalysis/coordinates/ParmEd.py | 22 +++++-------------- .../coordinates/test_parmed.py | 7 ++++++ 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/package/MDAnalysis/coordinates/ParmEd.py b/package/MDAnalysis/coordinates/ParmEd.py index 823bd2960df..bef281f9c31 100644 --- a/package/MDAnalysis/coordinates/ParmEd.py +++ b/package/MDAnalysis/coordinates/ParmEd.py @@ -177,7 +177,7 @@ def convert(self, obj): raise ValueError("Writing Timesteps to ParmEd " "objects is not supported") else: - raise_from(TypeError("No atoms found in obj argument"), None) + raise TypeError("No atoms found in obj argument") from None # Check for topology information missing_topology = [] @@ -212,21 +212,9 @@ def convert(self, obj): atom_kwargs = [] for atom, name, resname, xyz, vel in zip(ag_or_ts, names, resnames, positions, velocities): - try: - resid = atom.resid - except AttributeError: - resid = 1 - try: - mass = atom.mass - except AttributeError: - mass = 0 - try: - segid = atom.segid - except AttributeError: - segid = "X" - chain_seg = {'segid': segid} - akwargs = {'name': name, "mass": mass} - for attrname in ('charge', 'type', + akwargs = {'name': name} + chain_seg = {'segid': atom.segid} + for attrname in ('mass', 'charge', 'type', 'altLoc', 'tempfactor', 'occupancy', 'gbscreen', 'solventradius', 'nbindex', 'rmin', 'epsilon', 'rmin14', @@ -252,7 +240,7 @@ def convert(self, obj): chain_seg['inscode'] = atom.icode except AttributeError: pass - atom_kwargs.append((akwargs, resname, resid, chain_seg, xyz, vel)) + atom_kwargs.append((akwargs, resname, atom.resid, chain_seg, xyz, vel)) struct = pmd.Structure() diff --git a/testsuite/MDAnalysisTests/coordinates/test_parmed.py b/testsuite/MDAnalysisTests/coordinates/test_parmed.py index 0710186d304..5d7b3b6ec54 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_parmed.py +++ b/testsuite/MDAnalysisTests/coordinates/test_parmed.py @@ -296,3 +296,10 @@ def test_pass_ts_error(): with pytest.raises(ValueError, match=err): c = ParmEdConverter() c.convert(u.trajectory.ts) + + +def test_incorrect_object_passed_typeerror(): + err = "No atoms found in obj argument" + with pytest.raises(TypeError, match=err): + c = ParmEdConverter() + c.convert("🧁") \ No newline at end of file From 63fbaae3fc826c2ab899bfde6891390b40ce86eb Mon Sep 17 00:00:00 2001 From: Lily Wang Date: Sat, 3 Apr 2021 01:46:25 -0700 Subject: [PATCH 6/8] remove emoji --- testsuite/MDAnalysisTests/coordinates/test_parmed.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testsuite/MDAnalysisTests/coordinates/test_parmed.py b/testsuite/MDAnalysisTests/coordinates/test_parmed.py index 5d7b3b6ec54..df65ac8c477 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_parmed.py +++ b/testsuite/MDAnalysisTests/coordinates/test_parmed.py @@ -302,4 +302,4 @@ def test_incorrect_object_passed_typeerror(): err = "No atoms found in obj argument" with pytest.raises(TypeError, match=err): c = ParmEdConverter() - c.convert("🧁") \ No newline at end of file + c.convert("we still don't support emojis :(") From 986a416f53ea25bc81d36704f02d9905bb7d2c33 Mon Sep 17 00:00:00 2001 From: Lily Wang Date: Sat, 3 Apr 2021 11:17:58 -0700 Subject: [PATCH 7/8] raise from -> raise_from --- package/MDAnalysis/coordinates/ParmEd.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package/MDAnalysis/coordinates/ParmEd.py b/package/MDAnalysis/coordinates/ParmEd.py index bef281f9c31..f08d007e999 100644 --- a/package/MDAnalysis/coordinates/ParmEd.py +++ b/package/MDAnalysis/coordinates/ParmEd.py @@ -76,6 +76,7 @@ import functools import itertools import warnings +from six import raise_from from . import base from ..topology.tables import SYMB2Z @@ -177,7 +178,7 @@ def convert(self, obj): raise ValueError("Writing Timesteps to ParmEd " "objects is not supported") else: - raise TypeError("No atoms found in obj argument") from None + raise_from(TypeError("No atoms found in obj argument"), None) # Check for topology information missing_topology = [] From 2d5e901eb74ba0a6aebc27f3ec39fa46b83c76ad Mon Sep 17 00:00:00 2001 From: Lily Wang Date: Sat, 3 Apr 2021 11:33:08 -0700 Subject: [PATCH 8/8] update CHANGELOG --- package/CHANGELOG | 2 ++ 1 file changed, 2 insertions(+) diff --git a/package/CHANGELOG b/package/CHANGELOG index aca247fe2e5..e59c09023c6 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -31,6 +31,8 @@ Changes * Maximum pinned versions in setup.py removed for python 3.6+ (PR #3139) Deprecations + * ParmEdConverter no longer accepts Timestep objects at all + (Issue #3031, PR #3172) * NCDFWriter `scale_factor` writing will change in version 2.0 to better match AMBER outputs (Issue #2327) * Deprecated using the last letter of the segid as the