diff --git a/package/AUTHORS b/package/AUTHORS index 94a3de28fd7..52e17f7c20d 100644 --- a/package/AUTHORS +++ b/package/AUTHORS @@ -242,6 +242,7 @@ Chronological list of authors - Valerij Talagayev - Kurt McKee - Fabian Zills + - Laksh Krishna Sharma diff --git a/package/CHANGELOG b/package/CHANGELOG index ee9e88cf86a..f40873bf8f6 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -17,7 +17,7 @@ The rules for this file: ??/??/?? IAlibay, HeetVekariya, marinegor, lilyminium, RMeli, ljwoods2, aditya292002, pstaerk, PicoCentauri, BFedder, tyler.je.reddy, SampurnaM, leonwehrhan, kainszs, orionarcher, - yuxuanzhuang, PythonFZ + yuxuanzhuang, PythonFZ, laksh-krishna-sharma * 2.8.0 @@ -52,6 +52,7 @@ Fixes * Fix groups.py doctests using sphinx directives (Issue #3925, PR #4374) Enhancements + * Improve error message for `AtomGroup.unwrap()` when bonds are not present.(Issue #4436, PR #4642) * Add `analysis.DSSP` module for protein secondary structure assignment, based on [pydssp](https://github.com/ShintaroMinami/PyDSSP) * Added a tqdm progress bar for `MDAnalysis.analysis.pca.PCA.transform()` (PR #4531) diff --git a/package/MDAnalysis/core/groups.py b/package/MDAnalysis/core/groups.py index 1cb57f1f342..c272f96f1e4 100644 --- a/package/MDAnalysis/core/groups.py +++ b/package/MDAnalysis/core/groups.py @@ -1874,9 +1874,13 @@ def unwrap(self, compound='fragments', reference='com', inplace=True): """ atoms = self.atoms # bail out early if no bonds in topology: - if not hasattr(atoms, 'bonds'): - raise NoDataError("{}.unwrap() not available; this requires Bonds" - "".format(self.__class__.__name__)) + if not hasattr(atoms, 'bonds'): + raise NoDataError( + f"{self.__class__.__name__}.unwrap() not available; this AtomGroup lacks defined bonds. " + "To resolve this, you can either:\n" + "1. Guess the bonds at universe creation using `guess_bonds = True`, or\n" + "2. Create a universe using a topology format where bonds are pre-defined." + ) unique_atoms = atoms.unsorted_unique # Parameter sanity checking diff --git a/testsuite/MDAnalysisTests/core/test_groups.py b/testsuite/MDAnalysisTests/core/test_groups.py index 0e15a1a9e32..e3400ea0f40 100644 --- a/testsuite/MDAnalysisTests/core/test_groups.py +++ b/testsuite/MDAnalysisTests/core/test_groups.py @@ -21,6 +21,7 @@ # J. Comput. Chem. 32 (2011), 2319--2327, doi:10.1002/jcc.21787 # import itertools +import re import numpy as np from numpy.testing import ( assert_array_equal, @@ -1434,46 +1435,52 @@ def test_component_set_plural(self, attr, groupname): setattr(comp, attr, 24) class TestAttributeGetting(object): - + @staticmethod @pytest.fixture() def universe(): return make_Universe(extras=('masses', 'altLocs')) + @staticmethod + @pytest.fixture() + def atoms(): + u = make_Universe(extras=("masses",), size=(3, 1, 1)) + return u.atoms + @pytest.mark.parametrize('attr', ['masses', 'altLocs']) def test_get_present_topattr_group(self, universe, attr): values = getattr(universe.atoms, attr) assert values is not None - + @pytest.mark.parametrize('attr', ['mass', 'altLoc']) def test_get_present_topattr_component(self, universe, attr): value = getattr(universe.atoms[0], attr) assert value is not None @pytest.mark.parametrize('attr,singular', [ - ('masses', 'mass'), + ('masses', 'mass'), ('altLocs', 'altLoc')]) def test_get_plural_topattr_from_component(self, universe, attr, singular): with pytest.raises(AttributeError) as exc: getattr(universe.atoms[0], attr) assert ('Do you mean ' + singular) in str(exc.value) - + @pytest.mark.parametrize('attr,singular', [ - ('masses', 'mass'), + ('masses', 'mass'), ('altLocs', 'altLoc')]) def test_get_sing_topattr_from_group(self, universe, attr, singular): with pytest.raises(AttributeError) as exc: getattr(universe.atoms, singular) - assert ('Do you mean '+attr) in str(exc.value) - + assert ('Do you mean ' + attr) in str(exc.value) + @pytest.mark.parametrize('attr,singular', [ - ('elements', 'element'), + ('elements', 'element'), ('tempfactors', 'tempfactor'), ('bonds', 'bonds')]) def test_get_absent_topattr_group(self, universe, attr, singular): with pytest.raises(NoDataError) as exc: getattr(universe.atoms, attr) - assert 'does not contain '+singular in str(exc.value) + assert 'does not contain ' + singular in str(exc.value) def test_get_non_topattr(self, universe): with pytest.raises(AttributeError) as exc: @@ -1481,11 +1488,15 @@ def test_get_non_topattr(self, universe): assert 'has no attribute' in str(exc.value) def test_unwrap_without_bonds(self, universe): - with pytest.raises(NoDataError) as exc: - universe.atoms.unwrap() - err = ('AtomGroup.unwrap() not available; ' - 'this requires Bonds') - assert str(exc.value) == err + expected_message = ( + "AtomGroup.unwrap() not available; this AtomGroup lacks defined bonds. " + "To resolve this, you can either:\n" + "1. Guess the bonds at universe creation using `guess_bonds = True`, or\n" + "2. Create a universe using a topology format where bonds are pre-defined." + ) + expected_message_pattern = re.escape(expected_message) + with pytest.raises(NoDataError, match=expected_message_pattern): + universe.atoms.unwrap() def test_get_absent_attr_method(self, universe): with pytest.raises(NoDataError) as exc: @@ -1512,7 +1523,7 @@ def test_attrmethod_wrong_group(self, universe): universe.atoms[0].center_of_mass() err = ('center_of_mass() is a method of AtomGroup, not Atom') assert str(exc.value) == err - + @pytest.mark.parametrize('attr', ['altlocs', 'alt_Locs']) def test_wrong_name(self, universe, attr): with pytest.raises(AttributeError) as exc: @@ -1521,7 +1532,6 @@ def test_wrong_name(self, universe, attr): 'Did you mean altLocs?').format(attr) assert str(exc.value) == err - class TestInitGroup(object): @staticmethod @pytest.fixture( diff --git a/testsuite/MDAnalysisTests/core/test_unwrap.py b/testsuite/MDAnalysisTests/core/test_unwrap.py index 8749ecae5bc..656d9b08144 100644 --- a/testsuite/MDAnalysisTests/core/test_unwrap.py +++ b/testsuite/MDAnalysisTests/core/test_unwrap.py @@ -21,6 +21,7 @@ # J. Comput. Chem. 32 (2011), 2319--2327, doi:10.1002/jcc.21787 # import numpy as np +import re from numpy.testing import (assert_raises, assert_almost_equal, assert_array_equal) import pytest @@ -336,7 +337,13 @@ def test_unwrap_no_bonds_exception_safety(self, level, compound, reference): group = group.segments # store original positions: orig_pos = group.atoms.positions - with pytest.raises(NoDataError): + error_message = ( + f"{group.__class__.__name__}.unwrap() not available; this AtomGroup lacks defined bonds. " + "To resolve this, you can either:\n" + "1. Guess the bonds at universe creation using `guess_bonds = True`, or\n" + "2. Create a universe using a topology format where bonds are pre-defined." + ) + with pytest.raises(NoDataError, match=re.escape(error_message)): group.unwrap(compound=compound, reference=reference, inplace=True) # make sure atom positions are unchanged: assert_array_equal(group.atoms.positions, orig_pos)