Improve error message when calling AtmGroup.unwrap() without bonds (#…

…4642)

* Fixes #4436
* improves the error message that is raised when calling AtomGroup.unwrap() without defined bonds. 
   The new message provides clearer guidance on how users can resolve the issue.
* update a test case to use pytest.raises() ability to check the error message
* Update AUTHORS and CHANGELOG for improved error message in AtomGroup.unwrap()

package/AUTHORS

@@ -242,6 +242,7 @@ Chronological list of authors
   - Valerij Talagayev
   - Kurt McKee
   - Fabian Zills
+  - Laksh Krishna Sharma
 
 
 

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)

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

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,58 +1435,68 @@ 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:
             universe.atoms.jabberwocky
         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(

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)