From 90a30747285883b87295437290ed81a01a399f7c Mon Sep 17 00:00:00 2001 From: tanii1125 Date: Tue, 9 Dec 2025 20:50:53 +0530 Subject: [PATCH 01/13] Fixes #5170: Improve msd_type parsing --- package/AUTHORS | 1 + package/MDAnalysis/analysis/msd.py | 8 +++++++- testsuite/MDAnalysisTests/analysis/test_msd.py | 14 +++++++++++++- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/package/AUTHORS b/package/AUTHORS index e23f2afcf2c..3b2f53c5265 100644 --- a/package/AUTHORS +++ b/package/AUTHORS @@ -265,6 +265,7 @@ Chronological list of authors - Raúl Lois-Cuns - Pranay Pelapkar - Shreejan Dolai + - Tanisha Dubey (@tanii1125) External code ------------- diff --git a/package/MDAnalysis/analysis/msd.py b/package/MDAnalysis/analysis/msd.py index 3ce9eecaa7e..e456fd93d15 100644 --- a/package/MDAnalysis/analysis/msd.py +++ b/package/MDAnalysis/analysis/msd.py @@ -382,7 +382,13 @@ def _parse_msd_type(self): "xyz": [0, 1, 2], } - self.msd_type = self.msd_type.lower() + # Validate type first + if not isinstance(self.msd_type, str): + raise TypeError("msd_type must be a string") + + # strip whitespace + lowercase + self.msd_type = self.msd_type.strip().lower() + try: self._dim = keys[self.msd_type] diff --git a/testsuite/MDAnalysisTests/analysis/test_msd.py b/testsuite/MDAnalysisTests/analysis/test_msd.py index 0e8c9dad532..91a3dae9590 100644 --- a/testsuite/MDAnalysisTests/analysis/test_msd.py +++ b/testsuite/MDAnalysisTests/analysis/test_msd.py @@ -122,7 +122,19 @@ def test_msdtype_error(self, u, SELECTION, msdtype): errmsg = f"invalid msd_type: {msdtype}" with pytest.raises(ValueError, match=errmsg): m = MSD(u, SELECTION, msd_type=msdtype) - + def test_msd_type_whitespace(self, u, SELECTION): + m = MSD(u,SELECTION, msd_type=" xy ", fft=False) + assert m.dim_fac == 2 + assert m._dim == [0,1] + def test_msd_type_uppercase(self, u, SELECTION): + m = MSD(u, SELECTION, msd_type=" Xz ", fft=False) + assert m.dim_fac == 2 + assert m._dim == [0, 2] + def test_msd_type_nonstring(self, u, SELECTION): + with pytest.raises(TypeError): + MSD(u, SELECTION, msd_type=123, fft=False) + + @pytest.mark.parametrize( "dim, dim_factor", [ From f720f0060c8184a47893d9984598368fc44aaa0b Mon Sep 17 00:00:00 2001 From: tanii1125 Date: Wed, 10 Dec 2025 22:47:07 +0530 Subject: [PATCH 02/13] Corrected black error --- package/MDAnalysis/analysis/msd.py | 1 - testsuite/MDAnalysisTests/analysis/test_msd.py | 10 ++++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/package/MDAnalysis/analysis/msd.py b/package/MDAnalysis/analysis/msd.py index e456fd93d15..a1c23f42608 100644 --- a/package/MDAnalysis/analysis/msd.py +++ b/package/MDAnalysis/analysis/msd.py @@ -389,7 +389,6 @@ def _parse_msd_type(self): # strip whitespace + lowercase self.msd_type = self.msd_type.strip().lower() - try: self._dim = keys[self.msd_type] except KeyError: diff --git a/testsuite/MDAnalysisTests/analysis/test_msd.py b/testsuite/MDAnalysisTests/analysis/test_msd.py index 91a3dae9590..1dba4942d0d 100644 --- a/testsuite/MDAnalysisTests/analysis/test_msd.py +++ b/testsuite/MDAnalysisTests/analysis/test_msd.py @@ -122,19 +122,21 @@ def test_msdtype_error(self, u, SELECTION, msdtype): errmsg = f"invalid msd_type: {msdtype}" with pytest.raises(ValueError, match=errmsg): m = MSD(u, SELECTION, msd_type=msdtype) + def test_msd_type_whitespace(self, u, SELECTION): - m = MSD(u,SELECTION, msd_type=" xy ", fft=False) + m = MSD(u, SELECTION, msd_type=" xy ", fft=False) assert m.dim_fac == 2 - assert m._dim == [0,1] + assert m._dim == [0, 1] + def test_msd_type_uppercase(self, u, SELECTION): m = MSD(u, SELECTION, msd_type=" Xz ", fft=False) assert m.dim_fac == 2 assert m._dim == [0, 2] + def test_msd_type_nonstring(self, u, SELECTION): with pytest.raises(TypeError): MSD(u, SELECTION, msd_type=123, fft=False) - - + @pytest.mark.parametrize( "dim, dim_factor", [ From 21bfdf810d4af369d3fe591d583a7baa3a619410 Mon Sep 17 00:00:00 2001 From: tanii1125 Date: Sat, 13 Dec 2025 16:16:43 +0530 Subject: [PATCH 03/13] Added Tanisha Dubey to authors --- package/AUTHORS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/AUTHORS b/package/AUTHORS index 3b2f53c5265..6c9b9fe93c4 100644 --- a/package/AUTHORS +++ b/package/AUTHORS @@ -265,7 +265,7 @@ Chronological list of authors - Raúl Lois-Cuns - Pranay Pelapkar - Shreejan Dolai - - Tanisha Dubey (@tanii1125) + - Tanisha Dubey External code ------------- From 206766d0e8c6e9f65349a463dcbc2e003b9e76da Mon Sep 17 00:00:00 2001 From: tanii1125 Date: Wed, 24 Dec 2025 12:46:37 +0530 Subject: [PATCH 04/13] Improve msd_type parsing: clearer errors and safer string handling --- package/MDAnalysis/analysis/msd.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/package/MDAnalysis/analysis/msd.py b/package/MDAnalysis/analysis/msd.py index a1c23f42608..1feb91266db 100644 --- a/package/MDAnalysis/analysis/msd.py +++ b/package/MDAnalysis/analysis/msd.py @@ -382,13 +382,16 @@ def _parse_msd_type(self): "xyz": [0, 1, 2], } - # Validate type first - if not isinstance(self.msd_type, str): - raise TypeError("msd_type must be a string") - - # strip whitespace + lowercase - self.msd_type = self.msd_type.strip().lower() - + # lowercase + try: + self.msd_type = self.msd_type.lower() + except AttributeError: + raise TypeError("msd_type must be a string-like") + + # check for empty string + if not self.msd_type.strip(): + raise ValueError("msd_type cannot be empty or whitespace") + try: self._dim = keys[self.msd_type] except KeyError: From 8a9b472e8487bb1f91bcdb8c22ccf9dbe77ec24f Mon Sep 17 00:00:00 2001 From: tanii1125 Date: Wed, 24 Dec 2025 12:48:04 +0530 Subject: [PATCH 05/13] Update tests for msd_type validation --- testsuite/MDAnalysisTests/analysis/test_msd.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/testsuite/MDAnalysisTests/analysis/test_msd.py b/testsuite/MDAnalysisTests/analysis/test_msd.py index 1dba4942d0d..4a84d52c5f7 100644 --- a/testsuite/MDAnalysisTests/analysis/test_msd.py +++ b/testsuite/MDAnalysisTests/analysis/test_msd.py @@ -123,13 +123,12 @@ def test_msdtype_error(self, u, SELECTION, msdtype): with pytest.raises(ValueError, match=errmsg): m = MSD(u, SELECTION, msd_type=msdtype) - def test_msd_type_whitespace(self, u, SELECTION): - m = MSD(u, SELECTION, msd_type=" xy ", fft=False) - assert m.dim_fac == 2 - assert m._dim == [0, 1] - + def test_msd_type_empty_string(self, u, SELECTION): + with pytest.raises(ValueError): + MSD(u, SELECTION, msd_type=" ", fft=False) + def test_msd_type_uppercase(self, u, SELECTION): - m = MSD(u, SELECTION, msd_type=" Xz ", fft=False) + m = MSD(u, SELECTION, msd_type="Xz", fft=False) assert m.dim_fac == 2 assert m._dim == [0, 2] @@ -137,6 +136,11 @@ def test_msd_type_nonstring(self, u, SELECTION): with pytest.raises(TypeError): MSD(u, SELECTION, msd_type=123, fft=False) + def test_msd_type_whitespace_around_valid_value(self, u, SELECTION): + with pytest.raises(ValueError): + MSD(u, SELECTION, msd_type=" xy ", fft=False) + + @pytest.mark.parametrize( "dim, dim_factor", [ From a746e0cacf2b662d0ff82747aa9984002127ed7f Mon Sep 17 00:00:00 2001 From: tanii1125 Date: Thu, 25 Dec 2025 11:30:01 +0530 Subject: [PATCH 06/13] Black: reformatted files --- package/MDAnalysis/analysis/msd.py | 24 +++++--------- .../MDAnalysisTests/analysis/test_msd.py | 31 +++++-------------- 2 files changed, 15 insertions(+), 40 deletions(-) diff --git a/package/MDAnalysis/analysis/msd.py b/package/MDAnalysis/analysis/msd.py index 1feb91266db..88c78979d12 100644 --- a/package/MDAnalysis/analysis/msd.py +++ b/package/MDAnalysis/analysis/msd.py @@ -336,9 +336,7 @@ def __init__( **kwargs, ): if isinstance(u, groups.UpdatingAtomGroup): - raise TypeError( - "UpdatingAtomGroups are not valid for MSD computation" - ) + raise TypeError("UpdatingAtomGroups are not valid for MSD computation") super(EinsteinMSD, self).__init__(u.universe.trajectory, **kwargs) @@ -362,12 +360,8 @@ def __init__( def _prepare(self): # self.n_frames only available here # these need to be zeroed prior to each run() call - self.results.msds_by_particle = np.zeros( - (self.n_frames, self.n_particles) - ) - self._position_array = np.zeros( - (self.n_frames, self.n_particles, self.dim_fac) - ) + self.results.msds_by_particle = np.zeros((self.n_frames, self.n_particles)) + self._position_array = np.zeros((self.n_frames, self.n_particles, self.dim_fac)) # self.results.timeseries not set here def _parse_msd_type(self): @@ -387,11 +381,11 @@ def _parse_msd_type(self): self.msd_type = self.msd_type.lower() except AttributeError: raise TypeError("msd_type must be a string-like") - + # check for empty string if not self.msd_type.strip(): raise ValueError("msd_type cannot be empty or whitespace") - + try: self._dim = keys[self.msd_type] except KeyError: @@ -406,9 +400,7 @@ def _single_frame(self): r"""Constructs array of positions for MSD calculation.""" # shape of position array set here, use span in last dimension # from this point on - self._position_array[self._frame_index] = self.ag.positions[ - :, self._dim - ] + self._position_array[self._frame_index] = self.ag.positions[:, self._dim] def _conclude(self): if self.non_linear: @@ -459,9 +451,7 @@ def _conclude_fft(self): # with FFT, np.float64 bit prescision required. verbose=self._verbose, desc="Calculating MSD with FFT per particle", ): - self.results.msds_by_particle[:, n] = tidynamics.msd( - positions[:, n, :] - ) + self.results.msds_by_particle[:, n] = tidynamics.msd(positions[:, n, :]) self.results.timeseries = self.results.msds_by_particle.mean(axis=1) self.results.delta_t_values = np.arange(self.n_frames) * ( self.times[1] - self.times[0] diff --git a/testsuite/MDAnalysisTests/analysis/test_msd.py b/testsuite/MDAnalysisTests/analysis/test_msd.py index 4a84d52c5f7..6297af380c4 100644 --- a/testsuite/MDAnalysisTests/analysis/test_msd.py +++ b/testsuite/MDAnalysisTests/analysis/test_msd.py @@ -126,7 +126,7 @@ def test_msdtype_error(self, u, SELECTION, msdtype): def test_msd_type_empty_string(self, u, SELECTION): with pytest.raises(ValueError): MSD(u, SELECTION, msd_type=" ", fft=False) - + def test_msd_type_uppercase(self, u, SELECTION): m = MSD(u, SELECTION, msd_type="Xz", fft=False) assert m.dim_fac == 2 @@ -140,7 +140,6 @@ def test_msd_type_whitespace_around_valid_value(self, u, SELECTION): with pytest.raises(ValueError): MSD(u, SELECTION, msd_type=" xy ", fft=False) - @pytest.mark.parametrize( "dim, dim_factor", [ @@ -153,9 +152,7 @@ def test_msd_type_whitespace_around_valid_value(self, u, SELECTION): ("z", 1), ], ) - def test_simple_step_traj_all_dims( - self, step_traj, NSTEP, dim, dim_factor - ): + def test_simple_step_traj_all_dims(self, step_traj, NSTEP, dim, dim_factor): # testing the "simple" algorithm on constant velocity trajectory # should fit the polynomial y=dim_factor*x**2 m_simple = MSD(step_traj, "all", msd_type=dim, fft=False) @@ -175,18 +172,14 @@ def test_simple_step_traj_all_dims( ("z", 1), ], ) - def test_simple_start_stop_step_all_dims( - self, step_traj, NSTEP, dim, dim_factor - ): + def test_simple_start_stop_step_all_dims(self, step_traj, NSTEP, dim, dim_factor): # testing the "simple" algorithm on constant velocity trajectory # test start stop step is working correctly m_simple = MSD(step_traj, "all", msd_type=dim, fft=False) m_simple.run(start=10, stop=1000, step=10) poly = characteristic_poly(NSTEP, dim_factor) # polynomial must take offset start into account - assert_almost_equal( - m_simple.results.timeseries, poly[0:990:10], decimal=4 - ) + assert_almost_equal(m_simple.results.timeseries, poly[0:990:10], decimal=4) def test_random_walk_u_simple(self, random_walk_u): # regress against random_walk test data @@ -281,18 +274,14 @@ def test_fft_step_traj_all_dims(self, step_traj, NSTEP, dim, dim_factor): ("z", 1), ], ) - def test_fft_start_stop_step_all_dims( - self, step_traj, NSTEP, dim, dim_factor - ): + def test_fft_start_stop_step_all_dims(self, step_traj, NSTEP, dim, dim_factor): # testing the fft algorithm on constant velocity trajectory # test start stop step is working correctly m_simple = MSD(step_traj, "all", msd_type=dim, fft=True) m_simple.run(start=10, stop=1000, step=10) poly = characteristic_poly(NSTEP, dim_factor) # polynomial must take offset start into account - assert_almost_equal( - m_simple.results.timeseries, poly[0:990:10], decimal=3 - ) + assert_almost_equal(m_simple.results.timeseries, poly[0:990:10], decimal=3) def test_random_walk_u_fft(self, random_walk_u): # regress against random_walk test data @@ -1772,9 +1761,7 @@ def test_msds_per_particle(self, u_nonlinear): ] ) assert result_msd_per_particle.shape == expected_msd_per_particle.shape - assert_allclose( - result_msd_per_particle, expected_msd_per_particle, rtol=1e-5 - ) + assert_allclose(result_msd_per_particle, expected_msd_per_particle, rtol=1e-5) def test_start_stop_step(self, u_nonlinear): msd = MSD(u_nonlinear, select="all", msd_type="xyz", non_linear=True) @@ -1861,6 +1848,4 @@ def test_start_stop_step(self, u_nonlinear): assert result_msd_per_particle.shape == expected_msd_per_particle.shape assert_allclose(result_msd, expected_msd, rtol=1e-5) assert_allclose(result_delta_t, expected_delta_t, rtol=1e-5) - assert_allclose( - result_msd_per_particle, expected_msd_per_particle, rtol=1e-5 - ) + assert_allclose(result_msd_per_particle, expected_msd_per_particle, rtol=1e-5) From 2c69667ed5c8e484923aa169c1d39c54afd58c34 Mon Sep 17 00:00:00 2001 From: tanii1125 Date: Thu, 25 Dec 2025 14:27:39 +0530 Subject: [PATCH 07/13] Apply Black formatting --- .../MDAnalysisTests/analysis/test_msd.py | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/testsuite/MDAnalysisTests/analysis/test_msd.py b/testsuite/MDAnalysisTests/analysis/test_msd.py index 6297af380c4..9c95b4013a0 100644 --- a/testsuite/MDAnalysisTests/analysis/test_msd.py +++ b/testsuite/MDAnalysisTests/analysis/test_msd.py @@ -152,7 +152,9 @@ def test_msd_type_whitespace_around_valid_value(self, u, SELECTION): ("z", 1), ], ) - def test_simple_step_traj_all_dims(self, step_traj, NSTEP, dim, dim_factor): + def test_simple_step_traj_all_dims( + self, step_traj, NSTEP, dim, dim_factor + ): # testing the "simple" algorithm on constant velocity trajectory # should fit the polynomial y=dim_factor*x**2 m_simple = MSD(step_traj, "all", msd_type=dim, fft=False) @@ -172,14 +174,18 @@ def test_simple_step_traj_all_dims(self, step_traj, NSTEP, dim, dim_factor): ("z", 1), ], ) - def test_simple_start_stop_step_all_dims(self, step_traj, NSTEP, dim, dim_factor): + def test_simple_start_stop_step_all_dims( + self, step_traj, NSTEP, dim, dim_factor + ): # testing the "simple" algorithm on constant velocity trajectory # test start stop step is working correctly m_simple = MSD(step_traj, "all", msd_type=dim, fft=False) m_simple.run(start=10, stop=1000, step=10) poly = characteristic_poly(NSTEP, dim_factor) # polynomial must take offset start into account - assert_almost_equal(m_simple.results.timeseries, poly[0:990:10], decimal=4) + assert_almost_equal( + m_simple.results.timeseries, poly[0:990:10], decimal=4 + ) def test_random_walk_u_simple(self, random_walk_u): # regress against random_walk test data @@ -274,14 +280,18 @@ def test_fft_step_traj_all_dims(self, step_traj, NSTEP, dim, dim_factor): ("z", 1), ], ) - def test_fft_start_stop_step_all_dims(self, step_traj, NSTEP, dim, dim_factor): + def test_fft_start_stop_step_all_dims( + self, step_traj, NSTEP, dim, dim_factor + ): # testing the fft algorithm on constant velocity trajectory # test start stop step is working correctly m_simple = MSD(step_traj, "all", msd_type=dim, fft=True) m_simple.run(start=10, stop=1000, step=10) poly = characteristic_poly(NSTEP, dim_factor) # polynomial must take offset start into account - assert_almost_equal(m_simple.results.timeseries, poly[0:990:10], decimal=3) + assert_almost_equal( + m_simple.results.timeseries, poly[0:990:10], decimal=3 + ) def test_random_walk_u_fft(self, random_walk_u): # regress against random_walk test data @@ -1761,7 +1771,9 @@ def test_msds_per_particle(self, u_nonlinear): ] ) assert result_msd_per_particle.shape == expected_msd_per_particle.shape - assert_allclose(result_msd_per_particle, expected_msd_per_particle, rtol=1e-5) + assert_allclose( + result_msd_per_particle, expected_msd_per_particle, rtol=1e-5 + ) def test_start_stop_step(self, u_nonlinear): msd = MSD(u_nonlinear, select="all", msd_type="xyz", non_linear=True) @@ -1848,4 +1860,6 @@ def test_start_stop_step(self, u_nonlinear): assert result_msd_per_particle.shape == expected_msd_per_particle.shape assert_allclose(result_msd, expected_msd, rtol=1e-5) assert_allclose(result_delta_t, expected_delta_t, rtol=1e-5) - assert_allclose(result_msd_per_particle, expected_msd_per_particle, rtol=1e-5) + assert_allclose( + result_msd_per_particle, expected_msd_per_particle, rtol=1e-5 + ) From ced5eced04bba08266cb3c70ee8b466d9170e794 Mon Sep 17 00:00:00 2001 From: tanii1125 Date: Fri, 26 Dec 2025 18:35:03 +0530 Subject: [PATCH 08/13] black-formatting-issue --- package/MDAnalysis/analysis/msd.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/package/MDAnalysis/analysis/msd.py b/package/MDAnalysis/analysis/msd.py index 88c78979d12..2d51fffda70 100644 --- a/package/MDAnalysis/analysis/msd.py +++ b/package/MDAnalysis/analysis/msd.py @@ -336,7 +336,9 @@ def __init__( **kwargs, ): if isinstance(u, groups.UpdatingAtomGroup): - raise TypeError("UpdatingAtomGroups are not valid for MSD computation") + raise TypeError( + "UpdatingAtomGroups are not valid for MSD computation" + ) super(EinsteinMSD, self).__init__(u.universe.trajectory, **kwargs) @@ -360,8 +362,12 @@ def __init__( def _prepare(self): # self.n_frames only available here # these need to be zeroed prior to each run() call - self.results.msds_by_particle = np.zeros((self.n_frames, self.n_particles)) - self._position_array = np.zeros((self.n_frames, self.n_particles, self.dim_fac)) + self.results.msds_by_particle = np.zeros( + (self.n_frames, self.n_particles) + ) + self._position_array = np.zeros( + (self.n_frames, self.n_particles, self.dim_fac) + ) # self.results.timeseries not set here def _parse_msd_type(self): @@ -400,7 +406,9 @@ def _single_frame(self): r"""Constructs array of positions for MSD calculation.""" # shape of position array set here, use span in last dimension # from this point on - self._position_array[self._frame_index] = self.ag.positions[:, self._dim] + self._position_array[self._frame_index] = self.ag.positions[ + :, self._dim + ] def _conclude(self): if self.non_linear: @@ -451,7 +459,9 @@ def _conclude_fft(self): # with FFT, np.float64 bit prescision required. verbose=self._verbose, desc="Calculating MSD with FFT per particle", ): - self.results.msds_by_particle[:, n] = tidynamics.msd(positions[:, n, :]) + self.results.msds_by_particle[:, n] = tidynamics.msd( + positions[:, n, :] + ) self.results.timeseries = self.results.msds_by_particle.mean(axis=1) self.results.delta_t_values = np.arange(self.n_frames) * ( self.times[1] - self.times[0] From 45a46584fb975e802ca1d581a8a039187e805ef8 Mon Sep 17 00:00:00 2001 From: tanii1125 Date: Sat, 3 Jan 2026 19:57:04 +0530 Subject: [PATCH 09/13] Improve msd_type error handling with minimal changes --- package/MDAnalysis/analysis/msd.py | 12 ++---------- testsuite/MDAnalysisTests/analysis/test_msd.py | 8 -------- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/package/MDAnalysis/analysis/msd.py b/package/MDAnalysis/analysis/msd.py index 2d51fffda70..10c7db8545f 100644 --- a/package/MDAnalysis/analysis/msd.py +++ b/package/MDAnalysis/analysis/msd.py @@ -382,18 +382,10 @@ def _parse_msd_type(self): "xyz": [0, 1, 2], } - # lowercase try: - self.msd_type = self.msd_type.lower() + self._dim = keys[self.msd_type.lower()] except AttributeError: - raise TypeError("msd_type must be a string-like") - - # check for empty string - if not self.msd_type.strip(): - raise ValueError("msd_type cannot be empty or whitespace") - - try: - self._dim = keys[self.msd_type] + raise TypeError("msd_type must be a string") except KeyError: raise ValueError( "invalid msd_type: {} specified, please specify one of xyz, " diff --git a/testsuite/MDAnalysisTests/analysis/test_msd.py b/testsuite/MDAnalysisTests/analysis/test_msd.py index 9c95b4013a0..99a39c32617 100644 --- a/testsuite/MDAnalysisTests/analysis/test_msd.py +++ b/testsuite/MDAnalysisTests/analysis/test_msd.py @@ -123,10 +123,6 @@ def test_msdtype_error(self, u, SELECTION, msdtype): with pytest.raises(ValueError, match=errmsg): m = MSD(u, SELECTION, msd_type=msdtype) - def test_msd_type_empty_string(self, u, SELECTION): - with pytest.raises(ValueError): - MSD(u, SELECTION, msd_type=" ", fft=False) - def test_msd_type_uppercase(self, u, SELECTION): m = MSD(u, SELECTION, msd_type="Xz", fft=False) assert m.dim_fac == 2 @@ -136,10 +132,6 @@ def test_msd_type_nonstring(self, u, SELECTION): with pytest.raises(TypeError): MSD(u, SELECTION, msd_type=123, fft=False) - def test_msd_type_whitespace_around_valid_value(self, u, SELECTION): - with pytest.raises(ValueError): - MSD(u, SELECTION, msd_type=" xy ", fft=False) - @pytest.mark.parametrize( "dim, dim_factor", [ From 5f6286673ab7ce045bb72fa62d880a16bcec88ce Mon Sep 17 00:00:00 2001 From: tanii1125 Date: Sat, 3 Jan 2026 23:10:45 +0530 Subject: [PATCH 10/13] Improve msd_type error handling --- package/CHANGELOG | 2 +- package/MDAnalysis/analysis/msd.py | 9 ++---- .../MDAnalysisTests/analysis/test_msd.py | 29 ++++++++++--------- 3 files changed, 19 insertions(+), 21 deletions(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index 642e717b851..e51a441d98d 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -15,7 +15,7 @@ The rules for this file: ------------------------------------------------------------------------------- ??/??/?? IAlibay, orbeckst, marinegor, tylerjereddy, ljwoods2, marinegor, - spyke7, talagayev + spyke7, talagayev, tanii1125 * 2.11.0 diff --git a/package/MDAnalysis/analysis/msd.py b/package/MDAnalysis/analysis/msd.py index 10c7db8545f..33e95281a08 100644 --- a/package/MDAnalysis/analysis/msd.py +++ b/package/MDAnalysis/analysis/msd.py @@ -384,12 +384,9 @@ def _parse_msd_type(self): try: self._dim = keys[self.msd_type.lower()] - except AttributeError: - raise TypeError("msd_type must be a string") - except KeyError: - raise ValueError( - "invalid msd_type: {} specified, please specify one of xyz, " - "xy, xz, yz, x, y, z".format(self.msd_type) + except (AttributeError, KeyError): + raise TypeError( + "msd_type must be a string and one of: xyz, xy, xz, yz, x, y, z" ) self.dim_fac = len(self._dim) diff --git a/testsuite/MDAnalysisTests/analysis/test_msd.py b/testsuite/MDAnalysisTests/analysis/test_msd.py index 99a39c32617..65a4d9637e1 100644 --- a/testsuite/MDAnalysisTests/analysis/test_msd.py +++ b/testsuite/MDAnalysisTests/analysis/test_msd.py @@ -117,20 +117,21 @@ def test_updating_ag_rejected(self, u): with pytest.raises(TypeError, match=errmsg): m = MSD(updating_ag, msd_type="xyz", fft=False) - @pytest.mark.parametrize("msdtype", ["foo", "bar", "yx", "zyx"]) - def test_msdtype_error(self, u, SELECTION, msdtype): - errmsg = f"invalid msd_type: {msdtype}" - with pytest.raises(ValueError, match=errmsg): - m = MSD(u, SELECTION, msd_type=msdtype) - - def test_msd_type_uppercase(self, u, SELECTION): - m = MSD(u, SELECTION, msd_type="Xz", fft=False) - assert m.dim_fac == 2 - assert m._dim == [0, 2] - - def test_msd_type_nonstring(self, u, SELECTION): - with pytest.raises(TypeError): - MSD(u, SELECTION, msd_type=123, fft=False) + @pytest.mark.parametrize( + "msd_type, exc", + [ + ("Xz", None), # valid, mixed case. + (123, TypeError), # non-string. + ], + ) + def test_msdtype_error(self, u, SELECTION, msd_type, exc): + if exc is None: + m = MSD(u, SELECTION, msd_type=msd_type, fft=False) + assert m.dim_fac == 2 + assert m._dim == [0, 2] + else: + with pytest.raises(exc): + MSD(u, SELECTION, msd_type=msd_type, fft=False) @pytest.mark.parametrize( "dim, dim_factor", From b2bec49132b22598eeecfab17794697007c3634c Mon Sep 17 00:00:00 2001 From: tanii1125 Date: Sun, 4 Jan 2026 16:16:18 +0530 Subject: [PATCH 11/13] Improve msd_type error handling --- package/MDAnalysis/analysis/msd.py | 4 ++-- testsuite/MDAnalysisTests/analysis/test_msd.py | 18 +++++------------- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/package/MDAnalysis/analysis/msd.py b/package/MDAnalysis/analysis/msd.py index 33e95281a08..3b7af30660e 100644 --- a/package/MDAnalysis/analysis/msd.py +++ b/package/MDAnalysis/analysis/msd.py @@ -385,8 +385,8 @@ def _parse_msd_type(self): try: self._dim = keys[self.msd_type.lower()] except (AttributeError, KeyError): - raise TypeError( - "msd_type must be a string and one of: xyz, xy, xz, yz, x, y, z" + raise ValueError( + f"Invalid msd_type {self.msd_type.lower()} must be a string and one of: xyz, xy, xz, yz, x, y, z" ) self.dim_fac = len(self._dim) diff --git a/testsuite/MDAnalysisTests/analysis/test_msd.py b/testsuite/MDAnalysisTests/analysis/test_msd.py index 65a4d9637e1..3361ebd1a50 100644 --- a/testsuite/MDAnalysisTests/analysis/test_msd.py +++ b/testsuite/MDAnalysisTests/analysis/test_msd.py @@ -118,20 +118,12 @@ def test_updating_ag_rejected(self, u): m = MSD(updating_ag, msd_type="xyz", fft=False) @pytest.mark.parametrize( - "msd_type, exc", - [ - ("Xz", None), # valid, mixed case. - (123, TypeError), # non-string. - ], + "msdtype", ["foo", "bar", "yx", "zyx", 123, "", " xy "] ) - def test_msdtype_error(self, u, SELECTION, msd_type, exc): - if exc is None: - m = MSD(u, SELECTION, msd_type=msd_type, fft=False) - assert m.dim_fac == 2 - assert m._dim == [0, 2] - else: - with pytest.raises(exc): - MSD(u, SELECTION, msd_type=msd_type, fft=False) + def test_msdtype_error(self, u, SELECTION, msdtype): + errmsg = f"invalid msd_type: {msdtype}" + with pytest.raises(ValueError, match=errmsg): + m = MSD(u, SELECTION, msd_type=msdtype) @pytest.mark.parametrize( "dim, dim_factor", From fad64a3289ae61509dd8691b11a73968c7beabf7 Mon Sep 17 00:00:00 2001 From: tanii1125 Date: Sun, 4 Jan 2026 16:24:00 +0530 Subject: [PATCH 12/13] minor fix --- package/MDAnalysis/analysis/msd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/MDAnalysis/analysis/msd.py b/package/MDAnalysis/analysis/msd.py index 3b7af30660e..3f3b95fda84 100644 --- a/package/MDAnalysis/analysis/msd.py +++ b/package/MDAnalysis/analysis/msd.py @@ -386,7 +386,7 @@ def _parse_msd_type(self): self._dim = keys[self.msd_type.lower()] except (AttributeError, KeyError): raise ValueError( - f"Invalid msd_type {self.msd_type.lower()} must be a string and one of: xyz, xy, xz, yz, x, y, z" + f"Invalid msd_type {self.msd_type.lower()}, must be a string and one of: xyz, xy, xz, yz, x, y, z" ) self.dim_fac = len(self._dim) From 83594a4408b2e6e1d6db75d1567e9cc2d98e17e6 Mon Sep 17 00:00:00 2001 From: tanii1125 Date: Sun, 4 Jan 2026 17:33:34 +0530 Subject: [PATCH 13/13] improved test_msd.py --- package/MDAnalysis/analysis/msd.py | 2 +- testsuite/MDAnalysisTests/analysis/test_msd.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package/MDAnalysis/analysis/msd.py b/package/MDAnalysis/analysis/msd.py index 3f3b95fda84..03fc6b00109 100644 --- a/package/MDAnalysis/analysis/msd.py +++ b/package/MDAnalysis/analysis/msd.py @@ -386,7 +386,7 @@ def _parse_msd_type(self): self._dim = keys[self.msd_type.lower()] except (AttributeError, KeyError): raise ValueError( - f"Invalid msd_type {self.msd_type.lower()}, must be a string and one of: xyz, xy, xz, yz, x, y, z" + f"Invalid msd_type {self.msd_type}, must be a string and one of: xyz, xy, xz, yz, x, y, z" ) self.dim_fac = len(self._dim) diff --git a/testsuite/MDAnalysisTests/analysis/test_msd.py b/testsuite/MDAnalysisTests/analysis/test_msd.py index 3361ebd1a50..4a0a5717a38 100644 --- a/testsuite/MDAnalysisTests/analysis/test_msd.py +++ b/testsuite/MDAnalysisTests/analysis/test_msd.py @@ -121,7 +121,7 @@ def test_updating_ag_rejected(self, u): "msdtype", ["foo", "bar", "yx", "zyx", 123, "", " xy "] ) def test_msdtype_error(self, u, SELECTION, msdtype): - errmsg = f"invalid msd_type: {msdtype}" + errmsg = f"Invalid msd_type {msdtype}, must be a string and one of: xyz, xy, xz, yz, x, y, z" with pytest.raises(ValueError, match=errmsg): m = MSD(u, SELECTION, msd_type=msdtype)