Update _donor, _hydrogens, and _acceptors AtomGroups in HBondAnalysis whenever the selection strings are changed. (#5018)

- add sel as property and update ag when changed
- add test and changelog
- fix #5010

---------

Co-authored-by: Irfan Alibay <IAlibay@users.noreply.github.com>
Co-authored-by: Oliver Beckstein <orbeckst@gmail.com>

Author: 3 people

package/CHANGELOG

@@ -14,11 +14,15 @@ The rules for this file:
 
 
 -------------------------------------------------------------------------------
-??/??/?? IAlibay, orbeckst, BHM-Bob, TRY-ER, Abdulrahman-PROG, pbuslaev
+??/??/?? IAlibay, orbeckst, BHM-Bob, TRY-ER, Abdulrahman-PROG, pbuslaev,
+         yuxuanzhuang
 
  * 2.10.0
 
 Fixes
+ * Fixes bug in `analysis/hydrogenbonds.py`: `_donors` and `_hydrogens`
+   were not updated when the `acceptors_sel` and `hydrogens_sel` were provided
+   after initialization. (Issue #5010, PR #5018)
  * Fixes bug in `analysis/gnm.py`: `closeContactGNMAnalysis`: correct the
    `residue_index_map` generation when selection is not `protein`.
    (Issue #4924, PR #4961)

package/MDAnalysis/analysis/hydrogenbonds/hbond_analysis.py

@@ -345,14 +345,18 @@ def __init__(self, universe,
         .. versionchanged:: 2.8.0
             Introduced :meth:`get_supported_backends` allowing for parallel execution on
             :mod:`multiprocessing` and :mod:`dask` backends.
+        .. versionchanged:: 2.10.0
+            The `donors_sel`, `hydrogens_sel`, and `acceptors_sel` are stored as properties
+            now and update the internal `_donor`, `_hydrogens`, and `_acceptors` when they
+            are modified.
         """
 
         self.u = universe
         self._trajectory = self.u.trajectory
 
-        self.donors_sel = donors_sel.strip() if donors_sel is not None else donors_sel
-        self.hydrogens_sel = hydrogens_sel.strip() if hydrogens_sel is not None else hydrogens_sel
-        self.acceptors_sel = acceptors_sel.strip() if acceptors_sel is not None else acceptors_sel
+        self._donors_sel = donors_sel.strip() if donors_sel is not None else donors_sel
+        self._hydrogens_sel = hydrogens_sel.strip() if hydrogens_sel is not None else hydrogens_sel
+        self._acceptors_sel = acceptors_sel.strip() if acceptors_sel is not None else acceptors_sel
 
         msg = ("{} is an empty selection string - no hydrogen bonds will "
                "be found. This may be intended, but please check your "
@@ -393,10 +397,10 @@ def __init__(self, universe,
         self.results.hbonds = None
 
         # Set atom selections if they have not been provided
-        if self.acceptors_sel is None:
-            self.acceptors_sel = self.guess_acceptors()
-        if self.hydrogens_sel is None:
-            self.hydrogens_sel = self.guess_hydrogens()
+        if self._acceptors_sel is None:
+            self._acceptors_sel = self.guess_acceptors()
+        if self._hydrogens_sel is None:
+            self._hydrogens_sel = self.guess_hydrogens()
 
         # Select atom groups
         self._acceptors = self.u.select_atoms(self.acceptors_sel,
@@ -984,3 +988,47 @@ def count_by_ids(self):
         unique_hbonds = unique_hbonds[unique_hbonds[:, 3].argsort()[::-1]]
 
         return unique_hbonds
+
+    @property
+    def donors_sel(self):
+        """Selection string for the hydrogen bond donor atoms.
+        
+        .. versionadded:: 2.10.0
+        """
+        return self._donors_sel
+
+    @donors_sel.setter
+    def donors_sel(self, value):
+        self._donors_sel = value
+        self._donors, self._hydrogens = self._get_dh_pairs()
+
+    @property
+    def hydrogens_sel(self):
+        """Selection string for the hydrogen bond hydrogen atoms.
+        
+        .. versionadded:: 2.10.0
+        """
+        return self._hydrogens_sel
+
+    @hydrogens_sel.setter
+    def hydrogens_sel(self, value):
+        self._hydrogens_sel = value
+        if self._hydrogens_sel is None:
+            self._hydrogens_sel = self.guess_hydrogens()
+        self._donors, self._hydrogens = self._get_dh_pairs()
+
+    @property
+    def acceptors_sel(self):
+        """Selection string for the hydrogen bond acceptor atoms.
+        
+        .. versionadded:: 2.10.0
+        """
+        return self._acceptors_sel
+
+    @acceptors_sel.setter
+    def acceptors_sel(self, value):
+        self._acceptors_sel = value
+        if self._acceptors_sel is None:
+            self._acceptors_sel = self.guess_acceptors()
+        self._acceptors = self.u.select_atoms(self._acceptors_sel,
+                                              updating=self.update_selections)

testsuite/MDAnalysisTests/analysis/test_hydrogenbonds_analysis.py

@@ -63,6 +63,22 @@ def h(self, universe, client_HydrogenBondAnalysis):
         h.run(**client_HydrogenBondAnalysis)
         return h
 
+    @pytest.fixture(scope="class")
+    def h_modified(self, universe, client_HydrogenBondAnalysis):
+        kwargs = self.kwargs.copy()
+        donors_sel = kwargs.pop("donors_sel")
+        hydrogens_sel = kwargs.pop("hydrogens_sel")
+        acceptors_sel = kwargs.pop("acceptors_sel")
+        h = HydrogenBondAnalysis(universe, **kwargs)
+        h.donors_sel = donors_sel
+        h.hydrogens_sel = hydrogens_sel
+        h.acceptors_sel = acceptors_sel
+        h.run(**client_HydrogenBondAnalysis)
+        return h
+
+    def test_hbond_modified_consistency(self, h, h_modified):
+        assert_array_equal(h.results.hbonds, h_modified.results.hbonds)
+
     def test_hbond_analysis(self, h):
 
         assert len(np.unique(h.results.hbonds[:, 0])) == 10