Skip to content
Merged
1 change: 1 addition & 0 deletions package/AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ Chronological list of authors
- Ra煤l Lois-Cuns
- Pranay Pelapkar
- Shreejan Dolai
- Tanisha Dubey

External code
-------------
Expand Down
2 changes: 1 addition & 1 deletion package/CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ The rules for this file:

-------------------------------------------------------------------------------
??/??/?? IAlibay, orbeckst, marinegor, tylerjereddy, ljwoods2, marinegor,
spyke7, talagayev
spyke7, talagayev, tanii1125

* 2.11.0

Expand Down
11 changes: 4 additions & 7 deletions package/MDAnalysis/analysis/msd.py
Original file line number Diff line number Diff line change
Expand Up @@ -382,14 +382,11 @@ def _parse_msd_type(self):
"xyz": [0, 1, 2],
}

self.msd_type = self.msd_type.lower()

try:
self._dim = keys[self.msd_type]
except KeyError:
raise ValueError(
"invalid msd_type: {} specified, please specify one of xyz, "
"xy, xz, yz, x, y, z".format(self.msd_type)
self._dim = keys[self.msd_type.lower()]
except (AttributeError, KeyError):
raise TypeError(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
raise TypeError(
raise ValueError(

Keep the value error, it's an error in the input value.

"msd_type must be a string and one of: xyz, xy, xz, yz, x, y, z"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"msd_type must be a string and one of: xyz, xy, xz, yz, x, y, z"
f"Invalid msd_type {msd_type} must be a string and one of: xyz, xy, xz, yz, x, y, z"

Keep the parameter exposed in the error message.

)

self.dim_fac = len(self._dim)
Expand Down
20 changes: 15 additions & 5 deletions testsuite/MDAnalysisTests/analysis/test_msd.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +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)
@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:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't do this, keep this solely to checking for an error being raised with bad inputs.

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)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't do this, keep all the old parameters ("foo", "bar", etc...) and solely check for the ValueError to be raised, just like the old test.


@pytest.mark.parametrize(
"dim, dim_factor",
Expand Down
Loading