From 00e823faa52fffaeae400e8481f001b4df2d3e83 Mon Sep 17 00:00:00 2001 From: Bansal Utkarsh Date: Mon, 21 Aug 2017 20:11:13 +0530 Subject: [PATCH 1/3] Remove yield based tests --- testsuite/MDAnalysisTests/coordinates/base.py | 40 ++++++++----------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/testsuite/MDAnalysisTests/coordinates/base.py b/testsuite/MDAnalysisTests/coordinates/base.py index 6fe6eab7c66..6291e322414 100644 --- a/testsuite/MDAnalysisTests/coordinates/base.py +++ b/testsuite/MDAnalysisTests/coordinates/base.py @@ -701,7 +701,7 @@ def test_forces_remove(self): with pytest.raises(NoDataError): getattr(ts, 'forces') - def _empty_ts(self): + def check_ts(self): with pytest.raises(ValueError): self.Timestep.from_coordinates(None, None, None) @@ -714,7 +714,12 @@ def _from_coords(self, p, v, f): return ts - def _check_from_coordinates(self, p, v, f): + @pytest.mark.parametrize('p, v, f', + [(p, v, f) for (p, v, f) in + itertools.product([True, False], repeat=3) + if any([p, v, f])] + ) + def test_from_coordinates(self, p, v, f): ts = self._from_coords(p, v, f) if p: @@ -733,15 +738,6 @@ def _check_from_coordinates(self, p, v, f): with pytest.raises(NoDataError): getattr(ts, 'forces') - def test_from_coordinates(self): - # Check all combinations of creating a Timestep from data - # 8 possibilites of with and without 3 data categories - for p, v, f in itertools.product([True, False], repeat=3): - if not any([p, v, f]): - yield self._empty_ts - else: - yield self._check_from_coordinates, p, v, f - def test_from_coordinates_mismatch(self): velo = self.refvel[:2] @@ -752,18 +748,17 @@ def test_from_coordinates_nodata(self): with pytest.raises(ValueError): self.Timestep.from_coordinates() - def _check_from_timestep(self, p, v, f): + @pytest.mark.parametrize('p, v, f', + [(p, v, f) for (p, v, f) in + itertools.product([True, False], repeat=3) + if any([p, v, f])] + ) + def test_from_timestep(self, p, v, f): ts = self._from_coords(p, v, f) ts2 = self.Timestep.from_timestep(ts) assert_timestep_almost_equal(ts, ts2) - def test_from_timestep(self): - for p, v, f in itertools.product([True, False], repeat=3): - if not any([p, v, f]): - continue - yield self._check_from_timestep, p, v, f - # Time related tests def test_supply_dt(self): # Check that this gets stored in data properly @@ -952,17 +947,14 @@ def test_copy_slice(self): yield self._check_copy_slice_indices, self.name, ts yield self._check_copy_slice_slice, self.name, ts - def _check_bad_slice(self, p, v, f): + @pytest.mark.parametrize('p, v, f', [(p, v, f) for (p, v, f) in itertools.product([True, False], repeat=3) + if any([p, v, f])]) + def test_bad_slice(self, p, v, f): ts = self._from_coords(p, v, f) sl = ['this', 'is', 'silly'] with pytest.raises(TypeError): ts.copy_slice(sl) - def test_bad_copy_slice(self): - for p, v, f in itertools.product([True, False], repeat=3): - if not any([p, v, f]): - continue - yield self._check_bad_slice, p, v, f def _get_pos(self): # Get generic reference positions From d3983fd14286c8483adfbd9e1aa56eb2880284f4 Mon Sep 17 00:00:00 2001 From: Max Linke Date: Sun, 3 Sep 2017 22:13:43 +0200 Subject: [PATCH 2/3] replace yield based tests use fixture for timestep instead of parametrize --- testsuite/MDAnalysisTests/coordinates/base.py | 113 ++++++++---------- 1 file changed, 53 insertions(+), 60 deletions(-) diff --git a/testsuite/MDAnalysisTests/coordinates/base.py b/testsuite/MDAnalysisTests/coordinates/base.py index 6291e322414..2ce188cf0fb 100644 --- a/testsuite/MDAnalysisTests/coordinates/base.py +++ b/testsuite/MDAnalysisTests/coordinates/base.py @@ -701,7 +701,7 @@ def test_forces_remove(self): with pytest.raises(NoDataError): getattr(ts, 'forces') - def check_ts(self): + def test_check_ts(self): with pytest.raises(ValueError): self.Timestep.from_coordinates(None, None, None) @@ -714,11 +714,9 @@ def _from_coords(self, p, v, f): return ts - @pytest.mark.parametrize('p, v, f', - [(p, v, f) for (p, v, f) in - itertools.product([True, False], repeat=3) - if any([p, v, f])] - ) + @pytest.mark.parametrize('p, v, f', filter(any, + itertools.product([True, False], + repeat=3))) def test_from_coordinates(self, p, v, f): ts = self._from_coords(p, v, f) @@ -748,17 +746,6 @@ def test_from_coordinates_nodata(self): with pytest.raises(ValueError): self.Timestep.from_coordinates() - @pytest.mark.parametrize('p, v, f', - [(p, v, f) for (p, v, f) in - itertools.product([True, False], repeat=3) - if any([p, v, f])] - ) - def test_from_timestep(self, p, v, f): - ts = self._from_coords(p, v, f) - ts2 = self.Timestep.from_timestep(ts) - - assert_timestep_almost_equal(ts, ts2) - # Time related tests def test_supply_dt(self): # Check that this gets stored in data properly @@ -937,58 +924,64 @@ def test_copy(self, func, ts): ts = u.trajectory.ts func(self, self.name, ts) - def test_copy_slice(self): - for p, v, f in itertools.product([True, False], repeat=3): - if not any([p, v, f]): - continue - ts = self._from_coords(p, v, f) - yield self._check_copy, self.name, ts - yield self._check_independent, self.name, ts - yield self._check_copy_slice_indices, self.name, ts - yield self._check_copy_slice_slice, self.name, ts - - @pytest.mark.parametrize('p, v, f', [(p, v, f) for (p, v, f) in itertools.product([True, False], repeat=3) - if any([p, v, f])]) - def test_bad_slice(self, p, v, f): - ts = self._from_coords(p, v, f) + @pytest.fixture(params=filter(any, + itertools.product([True, False], repeat=3))) + def some_ts(self, request): + p, v, f = request.param + return self._from_coords(p, v, f) + + @pytest.mark.parametrize('func', [ + _check_copy, + _check_independent, + _check_copy_slice_indices, + _check_copy_slice_slice, + _check_npint_slice + ]) + def test_copy_slice(self, func, some_ts): + func(self, self.name, some_ts) + + def test_bad_slice(self, some_ts): sl = ['this', 'is', 'silly'] with pytest.raises(TypeError): - ts.copy_slice(sl) + some_ts.copy_slice(sl) + def test_from_timestep(self, some_ts): + ts = some_ts + ts2 = self.Timestep.from_timestep(ts) + + assert_timestep_almost_equal(ts, ts2) def _get_pos(self): # Get generic reference positions return np.arange(30).reshape(10, 3) * 1.234 def _check_ts_equal(self, a, b, err_msg): - assert_(a == b, err_msg) - assert_(b == a, err_msg) - - def test_check_equal(self): - for p, v, f in itertools.product([True, False], repeat=3): - if not any([p, v, f]): - continue - - ts1 = self.Timestep(self.size, - positions=p, - velocities=v, - forces=f) - ts2 = self.Timestep(self.size, - positions=p, - velocities=v, - forces=f) - if p: - ts1.positions = self.refpos.copy() - ts2.positions = self.refpos.copy() - if v: - ts1.velocities = self.refvel.copy() - ts2.velocities = self.refvel.copy() - if f: - ts1.forces = self.reffor.copy() - ts2.forces = self.reffor.copy() - - yield (self._check_ts_equal, ts1, ts2, - 'Failed on {0}'.format(self.name)) + assert a == b, err_msg + assert b == a, err_msg + + @pytest.mark.parametrize('p, v, f', filter(any, + itertools.product([True, False], + repeat=3))) + def test_check_equal(self, p, v, f): + ts1 = self.Timestep(self.size, + positions=p, + velocities=v, + forces=f) + ts2 = self.Timestep(self.size, + positions=p, + velocities=v, + forces=f) + if p: + ts1.positions = self.refpos.copy() + ts2.positions = self.refpos.copy() + if v: + ts1.velocities = self.refvel.copy() + ts2.velocities = self.refvel.copy() + if f: + ts1.forces = self.reffor.copy() + ts2.forces = self.reffor.copy() + + self._check_ts_equal(ts1, ts2, '') def test_wrong_class_equality(self): ts1 = self.Timestep(self.size) From bf6df3641a11dbcfb21e38a7d8c3c35f4f918eb3 Mon Sep 17 00:00:00 2001 From: Max Linke Date: Mon, 4 Sep 2017 14:29:02 +0200 Subject: [PATCH 3/3] add assert_timestep_equal standalone function to replace _check_ts_equal --- testsuite/MDAnalysisTests/coordinates/base.py | 13 ++++++++----- .../coordinates/test_timestep_api.py | 4 ++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/testsuite/MDAnalysisTests/coordinates/base.py b/testsuite/MDAnalysisTests/coordinates/base.py index 2ce188cf0fb..e4b04be3762 100644 --- a/testsuite/MDAnalysisTests/coordinates/base.py +++ b/testsuite/MDAnalysisTests/coordinates/base.py @@ -955,10 +955,6 @@ def _get_pos(self): # Get generic reference positions return np.arange(30).reshape(10, 3) * 1.234 - def _check_ts_equal(self, a, b, err_msg): - assert a == b, err_msg - assert b == a, err_msg - @pytest.mark.parametrize('p, v, f', filter(any, itertools.product([True, False], repeat=3))) @@ -981,7 +977,7 @@ def test_check_equal(self, p, v, f): ts1.forces = self.reffor.copy() ts2.forces = self.reffor.copy() - self._check_ts_equal(ts1, ts2, '') + assert_timestep_equal(ts1, ts2) def test_wrong_class_equality(self): ts1 = self.Timestep(self.size) @@ -1081,6 +1077,13 @@ def test_check_wrong_forces_equality(self): assert_(ts2 != ts1) +def assert_timestep_equal(A, B, msg=''): + """ assert that two timesteps are exactly equal and commutative + """ + assert A == B, msg + assert B == A, msg + + def assert_timestep_almost_equal(A, B, decimal=6, verbose=True): if not isinstance(A, Timestep): raise AssertionError('A is not of type Timestep') diff --git a/testsuite/MDAnalysisTests/coordinates/test_timestep_api.py b/testsuite/MDAnalysisTests/coordinates/test_timestep_api.py index 732a04fb933..1aa07cbeb6e 100644 --- a/testsuite/MDAnalysisTests/coordinates/test_timestep_api.py +++ b/testsuite/MDAnalysisTests/coordinates/test_timestep_api.py @@ -37,7 +37,7 @@ PDBQT_input, PQR, PRM, TRJ, PRMncdf, NCDF, TRZ_psf, TRZ) -from MDAnalysisTests.coordinates.base import BaseTimestepTest +from MDAnalysisTests.coordinates.base import BaseTimestepTest, assert_timestep_equal import pytest # Can add in custom tests for a given Timestep here! @@ -54,7 +54,7 @@ def test_other_timestep(self, otherTS): ts1.positions = self._get_pos() ts2 = otherTS(10) ts2.positions = self._get_pos() - self._check_ts_equal(ts1, ts2, "Failed on {0}".format(otherTS)) + assert_timestep_equal(ts1, ts2, "Failed on {0}".format(otherTS)) # TODO: Merge this into generic Reader tests