renamed 'pbc' kwarg to 'wrap' for Group methods

package/CHANGELOG

@@ -52,6 +52,8 @@ Changes
     (NEP29)
 
 Deprecations
+  * Deprecated the 'pbc' kwarg for various Group methods, the 'wrap' kwarg
+    should be used instead. (Issue #1760)
 
 
 08/21/21 tylerjereddy, richardjgowers, IAlibay, hmacdope, orbeckst, cbouy,

package/MDAnalysis/core/groups.py

@@ -432,15 +432,27 @@ class _ImmutableBase(object):
     __new__ = object.__new__
 
 
-def check_pbc_and_unwrap(function):
-    """Decorator to raise ValueError when both 'pbc' and 'unwrap' are set to True.
-    """
+def _pbc_to_wrap(function):
+    """Raises deprecation warning if 'pbc' is set and assigns value to 'wrap'"""
+    @functools.wraps(function)
+    def wrapped(group, *args, **kwargs):
+        if kwargs.get('pbc', None) is not None:
+            warnings.warn("The 'pbc' kwarg has been deprecated and will be "
+                          "removed in version 3.0., "
+                          "please use 'wrap' instead",
+                          DeprecationWarning)
+            kwargs['wrap'] = kwargs.pop('pbc')
+
+        return function(group, *args, **kwargs)
+    return wrapped
+
+
+def check_wrap_and_unwrap(function):
+    """Raises ValueError when both 'wrap' and 'unwrap' are set to True"""
     @functools.wraps(function)
     def wrapped(group, *args, **kwargs):
-        if kwargs.get('compound') == 'group':
-            if kwargs.get('pbc') and kwargs.get('unwrap'):
-                raise ValueError(
-                    "both 'pbc' and 'unwrap' can not be set to true")
+        if kwargs.get('wrap') and kwargs.get('unwrap'):
+            raise ValueError("both 'wrap' and 'unwrap' can not be set to true")
         return function(group, *args, **kwargs)
     return wrapped
 
@@ -950,8 +962,9 @@ def _split_by_compound_indices(self, compound, stable_sort=False):
         return atom_masks, compound_masks, len(compound_sizes)
 
     @warn_if_not_unique
-    @check_pbc_and_unwrap
-    def center(self, weights, pbc=False, compound='group', unwrap=False):
+    @_pbc_to_wrap
+    @check_wrap_and_unwrap
+    def center(self, weights, wrap=False, unwrap=False, compound='group'):
         """Weighted center of (compounds of) the group
 
         Computes the weighted center of :class:`Atoms<Atom>` in the group.
@@ -966,26 +979,24 @@ def center(self, weights, pbc=False, compound='group', unwrap=False):
         weights : array_like or None
             Weights to be used. Setting `weights=None` is equivalent to passing
             identical weights for all atoms of the group.
-        pbc : bool, optional
+        wrap : bool, optional
             If ``True`` and `compound` is ``'group'``, move all atoms to the
-            primary unit cell before calculation. If ``True`` and `compound` is
-            ``'segments'``, ``'residues'``, ``'molecules'``, or ``'fragments'``,
-            the center of each compound will be calculated without moving any
+            primary unit cell before calculation.
+            If ``True`` and `compound` is not ``'group'`` the center of each
+            compound will be calculated without moving any
             :class:`Atoms<Atom>` to keep the compounds intact. Instead, the
             resulting position vectors will be moved to the primary unit cell
             after calculation. Default [``False``].
+        unwrap : bool, optional
+            If ``True``, compounds will be unwrapped before computing their
+             centers.
         compound : {'group', 'segments', 'residues', 'molecules', 'fragments'}, optional
             If ``'group'``, the weighted center of all atoms in the group will
             be returned as a single position vector. Else, the weighted centers
             of each :class:`Segment`, :class:`Residue`, molecule, or fragment
             will be returned as an array of position vectors, i.e. a 2d array.
             Note that, in any case, *only* the positions of :class:`Atoms<Atom>`
             *belonging to the group* will be taken into account.
-        unwrap : bool, optional
-            If ``True``, compounds will be unwrapped before computing their
-            centers. The position of the first atom of each compound will be
-            taken as the reference to unwrap from; as such, results may differ
-            for the same :class:`AtomGroup` if atoms are ordered differently.
 
         Returns
         -------
@@ -1028,6 +1039,9 @@ def center(self, weights, pbc=False, compound='group', unwrap=False):
             compounds
         .. versionchanged:: 0.20.0 Added `unwrap` parameter
         .. versionchanged:: 1.0.0 Removed flags affecting default behaviour
+        .. versionchanged::
+           2.1.0 Renamed `pbc` kwarg to `wrap`. `pbc` is still accepted but
+           is deprecated and will be removed in version 3.0.
         """
         atoms = self.atoms
 
@@ -1036,7 +1050,7 @@ def center(self, weights, pbc=False, compound='group', unwrap=False):
 
         comp = compound.lower()
         if comp == 'group':
-            if pbc:
+            if wrap:
                 coords = atoms.pack_into_box(inplace=False)
             elif unwrap:
                 coords = atoms.unwrap(
@@ -1084,13 +1098,14 @@ def center(self, weights, pbc=False, compound='group', unwrap=False):
                 _centers = (_coords * _weights[:, :, None]).sum(axis=1)
                 _centers /= _weights.sum(axis=1)[:, None]
             centers[compound_mask] = _centers
-        if pbc:
+        if wrap:
             centers = distances.apply_PBC(centers, atoms.dimensions)
         return centers
 
     @warn_if_not_unique
-    @check_pbc_and_unwrap
-    def center_of_geometry(self, pbc=False, compound='group', unwrap=False):
+    @_pbc_to_wrap
+    @check_wrap_and_unwrap
+    def center_of_geometry(self, wrap=False, unwrap=False, compound='group'):
         """Center of geometry of (compounds of) the group.
 
         Computes the center of geometry (a.k.a. centroid) of
@@ -1100,22 +1115,24 @@ def center_of_geometry(self, pbc=False, compound='group', unwrap=False):
 
         Parameters
         ----------
-        pbc : bool, optional
+        wrap : bool, optional
             If ``True`` and `compound` is ``'group'``, move all atoms to the
             primary unit cell before calculation. If ``True`` and `compound` is
             ``'segments'`` or ``'residues'``, the center of each compound will
             be calculated without moving any :class:`Atoms<Atom>` to keep the
             compounds intact. Instead, the resulting position vectors will be
             moved to the primary unit cell after calculation. Default False.
+        unwrap : bool, optional
+            If ``True``, compounds will be unwrapped before computing their
+            centers.
         compound : {'group', 'segments', 'residues', 'molecules', 'fragments'}, optional
             If ``'group'``, the center of geometry of all :class:`Atoms<Atom>`
-            in the group will be returned as a single position vector. Else, the
-            centers of geometry of each :class:`Segment` or :class:`Residue`
-            will be returned as an array of position vectors, i.e. a 2d array.
-            Note that, in any case, *only* the positions of :class:`Atoms<Atom>`
-            *belonging to the group* will be taken into account.
-        unwrap : bool, optional
-            If ``True``, compounds will be unwrapped before computing their centers.
+            in the group will be returned as a single position vector. Else,
+            the centers of geometry of each :class:`Segment` or
+            :class:`Residue` will be returned as an array of position vectors,
+            i.e. a 2d array. Note that, in any case, *only* the positions of
+            :class:`Atoms<Atom>` *belonging to the group* will be taken into
+            account.
 
         Returns
         -------
@@ -1134,8 +1151,11 @@ def center_of_geometry(self, pbc=False, compound='group', unwrap=False):
             compounds
         .. versionchanged:: 0.20.0 Added `unwrap` parameter
         .. versionchanged:: 1.0.0 Removed flags affecting default behaviour
+        .. versionchanged::
+           2.1.0 Renamed `pbc` kwarg to `wrap`. `pbc` is still accepted but
+           is deprecated and will be removed in version 3.0.
         """
-        return self.center(None, pbc=pbc, compound=compound, unwrap=unwrap)
+        return self.center(None, wrap=wrap, compound=compound, unwrap=unwrap)
 
     centroid = center_of_geometry
 
@@ -1251,7 +1271,8 @@ def accumulate(self, attribute, function=np.sum, compound='group'):
             accumulation[compound_mask] = _accumulation
         return accumulation
 
-    def bbox(self, pbc=False):
+    @_pbc_to_wrap
+    def bbox(self, wrap=False):
         """Return the bounding box of the selection.
 
         The lengths A,B,C of the orthorhombic enclosing box are ::
@@ -1261,7 +1282,7 @@ def bbox(self, pbc=False):
 
         Parameters
         ----------
-        pbc : bool, optional
+        wrap : bool, optional
             If ``True``, move all :class:`Atoms<Atom>` to the primary unit cell
             before calculation. [``False``]
 
@@ -1275,25 +1296,30 @@ def bbox(self, pbc=False):
         .. versionadded:: 0.7.2
         .. versionchanged:: 0.8 Added *pbc* keyword
         .. versionchanged:: 1.0.0 Removed flags affecting default behaviour
+        .. versionchanged::
+           2.1.0 Renamed `pbc` kwarg to `wrap`. `pbc` is still accepted but
+           is deprecated and will be removed in version 3.0.
         """
+        # TODO: Add unwrap/compounds treatment
         atomgroup = self.atoms
 
-        if pbc:
+        if wrap:
             x = atomgroup.pack_into_box(inplace=False)
         else:
             x = atomgroup.positions
 
         return np.array([x.min(axis=0), x.max(axis=0)])
 
-    def bsphere(self, pbc=False):
+    @_pbc_to_wrap
+    def bsphere(self, wrap=False):
         """Return the bounding sphere of the selection.
 
         The sphere is calculated relative to the
         :meth:`center of geometry<center_of_geometry>`.
 
         Parameters
         ----------
-        pbc : bool, optional
+        wrap : bool, optional
             If ``True``, move all atoms to the primary unit cell before
             calculation. [``False``]
 
@@ -1307,15 +1333,18 @@ def bsphere(self, pbc=False):
 
         .. versionadded:: 0.7.3
         .. versionchanged:: 0.8 Added *pbc* keyword
+        .. versionchanged::
+           2.1.0 Renamed `pbc` kwarg to `wrap`. `pbc` is still accepted but
+           is deprecated and will be removed in version 3.0.
         """
         atomgroup = self.atoms.unsorted_unique
 
-        if pbc:
+        if wrap:
             x = atomgroup.pack_into_box(inplace=False)
-            centroid = atomgroup.center_of_geometry(pbc=True)
+            centroid = atomgroup.center_of_geometry(wrap=True)
         else:
             x = atomgroup.positions
-            centroid = atomgroup.center_of_geometry(pbc=False)
+            centroid = atomgroup.center_of_geometry(wrap=False)
 
         R = np.sqrt(np.max(np.sum(np.square(x - centroid), axis=1)))