From 1b4c29f2da04b742548df48da9af0af6f4f1baaf Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Sun, 9 Feb 2020 15:41:06 +0000 Subject: [PATCH 1/8] failing test --- .../MDAnalysisTests/coordinates/test_pdb.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/testsuite/MDAnalysisTests/coordinates/test_pdb.py b/testsuite/MDAnalysisTests/coordinates/test_pdb.py index c9557d3c437..5bed39a27c5 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_pdb.py +++ b/testsuite/MDAnalysisTests/coordinates/test_pdb.py @@ -185,6 +185,10 @@ def universe(self): def universe2(self): return mda.Universe(PSF, DCD) + @pytest.fixture + def universe3(self): + return mda.Universe(PDB) + @pytest.fixture def outfile(self, tmpdir): return str(tmpdir.mkdir("PDBWriter").join('primitive-pdb-writer' + self.ext)) @@ -362,6 +366,27 @@ def test_segid_chainid(self, universe2, outfile): u_pdb = mda.Universe(outfile) assert u_pdb.segments.chainIDs[0][0] == ref_id + def test_stringio_outofrange(self, universe3): + """ + Check that when StrinIO is used, the correct out-of-range error for + coordinates is raised (instead of failing trying to removeStrinIO + as a file). + """ + + u = universe3 + + u.atoms.translate([-9999, -9999, -9999]) + + outstring = StringIO() + + with pytest.raises(ValueError) as error: + with mda.coordinates.PDB.PDBWriter(outstring) as writer: + writer.write(u.atoms) + + expectedmsg = "PDB files must have coordinate values between" + assert error.msg[:len(expectedmsg)] == expectedmsg + + class TestMultiPDBReader(object): @staticmethod @pytest.fixture(scope='class') From 62b18d99cb2253b4d6e34891bfe588879c46eae2 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Sun, 9 Feb 2020 15:41:24 +0000 Subject: [PATCH 2/8] check if file is StringIO --- package/MDAnalysis/coordinates/PDB.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/package/MDAnalysis/coordinates/PDB.py b/package/MDAnalysis/coordinates/PDB.py index cf3208d87ef..40ac624702d 100644 --- a/package/MDAnalysis/coordinates/PDB.py +++ b/package/MDAnalysis/coordinates/PDB.py @@ -678,6 +678,10 @@ def _check_pdb_coordinates(self): except OSError as err: if err.errno == errno.ENOENT: pass + except TypeError as err: + if isinstance(self.filename, StringIO): + pass + raise ValueError("PDB files must have coordinate values between " "{0:.3f} and {1:.3f} Angstroem: file writing was " "aborted.".format(self.pdb_coor_limits["min"], From 432ed4e89179b92fd271f153e1a33b4b753c856c Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Sun, 9 Feb 2020 15:46:08 +0000 Subject: [PATCH 3/8] changelog --- package/CHANGELOG | 2 ++ package/MDAnalysis/coordinates/PDB.py | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/package/CHANGELOG b/package/CHANGELOG index c4f2e4afa85..1914638f6d6 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -20,6 +20,8 @@ mm/dd/yy richardjgowers, kain88-de, lilyminium, p-j-smith, bdice, joaomcteixeira * 0.21.0 Fixes + * Handle exception when PDBWriter is trying to remove an invalid StringIO + (Issue #2512) * Clarifies density_from_Universe docs and adds user warning (Issue #2372) * Fix upstream deprecation of `matplotlib.axis.Tick` attributes in `MDAnalysis.analysis.psa` diff --git a/package/MDAnalysis/coordinates/PDB.py b/package/MDAnalysis/coordinates/PDB.py index 40ac624702d..81597dd89f8 100644 --- a/package/MDAnalysis/coordinates/PDB.py +++ b/package/MDAnalysis/coordinates/PDB.py @@ -653,6 +653,10 @@ def _check_pdb_coordinates(self): coordinates and closes the file. Raises :exc:`ValueError` if the coordinates fail the check. + + .. versionchanged: 1.0.0 + Check if :attr:`finename` is `StringIO` when attempting to remove + an invalid file. """ atoms = self.obj.atoms # make sure to use atoms (Issue 46) # can write from selection == Universe (Issue 49) From 70f29718a7feb43dc8311552478f39bb19e8a4a5 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Sun, 9 Feb 2020 15:59:19 +0000 Subject: [PATCH 4/8] small change --- package/MDAnalysis/coordinates/PDB.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package/MDAnalysis/coordinates/PDB.py b/package/MDAnalysis/coordinates/PDB.py index 81597dd89f8..cb5b1c71d44 100644 --- a/package/MDAnalysis/coordinates/PDB.py +++ b/package/MDAnalysis/coordinates/PDB.py @@ -655,8 +655,8 @@ def _check_pdb_coordinates(self): Raises :exc:`ValueError` if the coordinates fail the check. .. versionchanged: 1.0.0 - Check if :attr:`finename` is `StringIO` when attempting to remove - an invalid file. + Check if :attr:`filename` is `StringIO` when attempting to remove + a PDB file with invalid coordinates (Issue #2512) """ atoms = self.obj.atoms # make sure to use atoms (Issue 46) # can write from selection == Universe (Issue 49) @@ -682,7 +682,7 @@ def _check_pdb_coordinates(self): except OSError as err: if err.errno == errno.ENOENT: pass - except TypeError as err: + except TypeError: if isinstance(self.filename, StringIO): pass From 7850ff3c1292851e2307ec15e9706877728a4849 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Sun, 9 Feb 2020 16:49:56 +0000 Subject: [PATCH 5/8] fix typos --- testsuite/MDAnalysisTests/coordinates/test_pdb.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testsuite/MDAnalysisTests/coordinates/test_pdb.py b/testsuite/MDAnalysisTests/coordinates/test_pdb.py index 5bed39a27c5..85d699c2a33 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_pdb.py +++ b/testsuite/MDAnalysisTests/coordinates/test_pdb.py @@ -368,8 +368,8 @@ def test_segid_chainid(self, universe2, outfile): def test_stringio_outofrange(self, universe3): """ - Check that when StrinIO is used, the correct out-of-range error for - coordinates is raised (instead of failing trying to removeStrinIO + Check that when StringIO is used, the correct out-of-range error for + coordinates is raised (instead of failing trying to remove StringIO as a file). """ From e4520f858245cb18d09ed7cf743b61e1be92c6d8 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Sun, 9 Feb 2020 17:31:02 +0000 Subject: [PATCH 6/8] actually check exception message --- testsuite/MDAnalysisTests/coordinates/test_pdb.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/testsuite/MDAnalysisTests/coordinates/test_pdb.py b/testsuite/MDAnalysisTests/coordinates/test_pdb.py index 85d699c2a33..34792fbd05e 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_pdb.py +++ b/testsuite/MDAnalysisTests/coordinates/test_pdb.py @@ -383,8 +383,9 @@ def test_stringio_outofrange(self, universe3): with mda.coordinates.PDB.PDBWriter(outstring) as writer: writer.write(u.atoms) - expectedmsg = "PDB files must have coordinate values between" - assert error.msg[:len(expectedmsg)] == expectedmsg + expectedmsg = "PDB files must have coordinate values between" + + assert error.value.args[0][:len(expectedmsg)] == expectedmsg class TestMultiPDBReader(object): From 02c7eb4a994cf6d7cf999d786ba5414758d32696 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Sun, 9 Feb 2020 22:02:04 +0000 Subject: [PATCH 7/8] fix problem with exception catch --- package/MDAnalysis/coordinates/PDB.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/package/MDAnalysis/coordinates/PDB.py b/package/MDAnalysis/coordinates/PDB.py index cb5b1c71d44..4466b002c4d 100644 --- a/package/MDAnalysis/coordinates/PDB.py +++ b/package/MDAnalysis/coordinates/PDB.py @@ -682,9 +682,13 @@ def _check_pdb_coordinates(self): except OSError as err: if err.errno == errno.ENOENT: pass + else: + raise except TypeError: if isinstance(self.filename, StringIO): pass + else: + raise raise ValueError("PDB files must have coordinate values between " "{0:.3f} and {1:.3f} Angstroem: file writing was " From 6efe65abd1284ba6d515b33c8e0a429f88a57ef8 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Mon, 10 Feb 2020 13:42:41 +0000 Subject: [PATCH 8/8] use pytest match --- testsuite/MDAnalysisTests/coordinates/test_pdb.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/testsuite/MDAnalysisTests/coordinates/test_pdb.py b/testsuite/MDAnalysisTests/coordinates/test_pdb.py index 34792fbd05e..1bec2eb05f2 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_pdb.py +++ b/testsuite/MDAnalysisTests/coordinates/test_pdb.py @@ -379,14 +379,12 @@ def test_stringio_outofrange(self, universe3): outstring = StringIO() - with pytest.raises(ValueError) as error: + errmsg = "PDB files must have coordinate values between" + + with pytest.raises(ValueError, match=errmsg): with mda.coordinates.PDB.PDBWriter(outstring) as writer: writer.write(u.atoms) - expectedmsg = "PDB files must have coordinate values between" - - assert error.value.args[0][:len(expectedmsg)] == expectedmsg - class TestMultiPDBReader(object): @staticmethod