diff --git a/package/CHANGELOG b/package/CHANGELOG index ed2d34516cf..c0f3ea09c33 100644 --- a/package/CHANGELOG +++ b/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, diff --git a/package/MDAnalysis/core/groups.py b/package/MDAnalysis/core/groups.py index 5a46b3eb14f..dce6aa78295 100644 --- a/package/MDAnalysis/core/groups.py +++ b/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` in the group. @@ -966,14 +979,17 @@ 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` 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 @@ -981,11 +997,6 @@ def center(self, weights, pbc=False, compound='group', unwrap=False): 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` *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` 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` - 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` - *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` *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` to the primary unit cell before calculation. [``False``] @@ -1275,17 +1296,22 @@ 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 @@ -1293,7 +1319,7 @@ def bsphere(self, pbc=False): 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))) diff --git a/package/MDAnalysis/core/topologyattrs.py b/package/MDAnalysis/core/topologyattrs.py index 9821ae9ab27..c4d681bd053 100644 --- a/package/MDAnalysis/core/topologyattrs.py +++ b/package/MDAnalysis/core/topologyattrs.py @@ -55,7 +55,7 @@ from .groups import (ComponentBase, GroupBase, Atom, Residue, Segment, AtomGroup, ResidueGroup, SegmentGroup, - check_pbc_and_unwrap) + check_wrap_and_unwrap, _pbc_to_wrap) from .. import _TOPOLOGY_ATTRS, _TOPOLOGY_TRANSPLANTS, _TOPOLOGY_ATTRNAMES @@ -1208,11 +1208,11 @@ def chi1_selection(residue, n_name='N', ca_name='CA', cb_name='CB', 4-atom selection in the correct order. If no CB and/or CG is found then this method returns ``None``. - .. versionchanged:: 1.0.0 - Added arguments for flexible atom names and refactored code for - faster atom matching with boolean arrays. .. versionadded:: 0.7.5 + .. versionchanged:: 1.0.0 + Added arguments for flexible atom names and refactored code for + faster atom matching with boolean arrays. """ names = [n_name, ca_name, cb_name, cg_name] ags = [residue.atoms.select_atoms(f"name {n}") for n in names] @@ -1451,9 +1451,10 @@ def get_segments(self, sg): return masses @warn_if_not_unique - @check_pbc_and_unwrap - def center_of_mass(group, pbc=False, compound='group', unwrap=False): - r"""Center of mass of (compounds of) the group. + @_pbc_to_wrap + @check_wrap_and_unwrap + def center_of_mass(group, wrap=False, unwrap=False, compound='group'): + """Center of mass of (compounds of) the group. Computes the center of mass of :class:`Atoms` in the group. Centers of mass per :class:`Residue`, :class:`Segment`, molecule, or @@ -1464,14 +1465,17 @@ def center_of_mass(group, 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 + If ``True`` and `compound` is not ``group``, the centers of mass of each compound will be calculated without moving any atoms to keep the compounds intact. Instead, the resulting center-of-mass position vectors will be moved to the primary unit cell after calculation. + 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 mass of all atoms in the group will @@ -1481,8 +1485,6 @@ def center_of_mass(group, pbc=False, compound='group', unwrap=False): array. Note that, in any case, *only* the positions of :class:`Atoms` *belonging to the group* will be taken into account. - unwrap : bool, optional - If ``True``, compounds will be unwrapped before computing their centers. Returns ------- @@ -1496,18 +1498,24 @@ def center_of_mass(group, pbc=False, compound='group', unwrap=False): Note ---- - * This method can only be accessed if the underlying topology has - information about atomic masses. - - - .. versionchanged:: 0.8 Added `pbc` parameter - .. versionchanged:: 0.19.0 Added `compound` parameter - .. versionchanged:: 0.20.0 Added ``'molecules'`` and ``'fragments'`` - compounds - .. versionchanged:: 0.20.0 Added `unwrap` parameter + This method can only be accessed if the underlying topology has + information about atomic masses. + + + .. versionchanged:: 0.8 + Added `pbc` parameter + .. versionchanged:: 0.19.0 + Added `compound` parameter + .. versionchanged:: 0.20.0 + Added ``'molecules'`` and ``'fragments'`` compounds; + added `unwrap` parameter + .. 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 = group.atoms - return atoms.center(weights=atoms.masses, pbc=pbc, compound=compound, unwrap=unwrap) + return atoms.center(weights=atoms.masses, wrap=wrap, compound=compound, + unwrap=unwrap) transplants[GroupBase].append( ('center_of_mass', center_of_mass)) @@ -1549,33 +1557,78 @@ def total_mass(group, compound='group'): ('total_mass', total_mass)) @warn_if_not_unique - @check_pbc_and_unwrap - def moment_of_inertia(group, pbc=False, **kwargs): - """Tensor moment of inertia relative to center of mass as 3x3 numpy - array. + @_pbc_to_wrap + @check_wrap_and_unwrap + def moment_of_inertia(group, wrap=False, unwrap=False, compound="group"): + r"""Moment of inertia tensor relative to center of mass. Parameters ---------- - pbc : bool, optional - If ``True``, move all atoms within the primary unit cell before - calculation. [``False``] + wrap : bool, optional + If ``True`` and `compound` is ``'group'``, move all atoms to the + primary unit cell before calculation. + If ``True`` and `compound` is not ``group``, the + centers of mass of each compound will be calculated without moving + any atoms to keep the compounds intact. Instead, the resulting + center-of-mass position vectors will be moved to the primary unit + cell after calculation. + unwrap : bool, optional + If ``True``, compounds will be unwrapped before computing their + centers and tensor of inertia. + compound : {'group', 'segments', 'residues', 'molecules', 'fragments'},\ + optional + `compound` determines the behavior of `wrap`. + Note that, in any case, *only* the positions of :class:`Atoms` + *belonging to the group* will be taken into account. + + Returns + ------- + moment_of_inertia : numpy.ndarray + Moment of inertia tensor as a 3 x 3 numpy array. + + Notes + ----- + The moment of inertia tensor :math:`\mathsf{I}` is calculated for a group of + :math:`N` atoms with coordinates :math:`\mathbf{r}_i,\ 1 \le i \le N` + relative to its center of mass from the relative coordinates + .. math:: + \mathbf{r}'_i = \mathbf{r}_i - \frac{1}{\sum_{i=1}^{N} m_i} \sum_{i=1}^{N} m_i \mathbf{r}_i - .. versionchanged:: 0.8 Added *pbc* keyword - .. versionchanged:: 0.20.0 Added `unwrap` parameter + as + .. math:: + \mathsf{I} = \sum_{i=1}^{N} m_i \Big[(\mathbf{r}'_i\cdot\mathbf{r}'_i) \sum_{\alpha=1}^{3} + \hat{\mathbf{e}}_\alpha \otimes \hat{\mathbf{e}}_\alpha - \mathbf{r}'_i \otimes \mathbf{r}'_i\Big] + + where :math:`\hat{\mathbf{e}}_\alpha` are Cartesian unit vectors, or in Cartesian coordinates + + .. math:: + I_{\alpha,\beta} = \sum_{k=1}^{N} m_k + \Big(\big(\sum_{\gamma=1}^3 (x'^{(k)}_{\gamma})^2 \big)\delta_{\alpha,\beta} + - x'^{(k)}_{\alpha} x'^{(k)}_{\beta} \Big). + + where :math:`x'^{(k)}_{\alpha}` are the Cartesian coordinates of the + relative coordinates :math:`\mathbf{r}'_k`. + + + .. versionchanged:: 0.8 + Added `pbc` keyword + .. versionchanged:: 0.20.0 + Added `unwrap` parameter + .. 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 = group.atoms - unwrap = kwargs.pop('unwrap', False) - compound = kwargs.pop('compound', 'group') com = atomgroup.center_of_mass( - pbc=pbc, unwrap=unwrap, compound=compound) + wrap=wrap, unwrap=unwrap, compound=compound) if compound != 'group': com = (com * group.masses[:, None] ).sum(axis=0) / group.masses.sum() - if pbc: + if wrap: pos = atomgroup.pack_into_box(inplace=False) - com elif unwrap: pos = atomgroup.unwrap(compound=compound, inplace=False) - com @@ -1612,24 +1665,28 @@ def moment_of_inertia(group, pbc=False, **kwargs): ('moment_of_inertia', moment_of_inertia)) @warn_if_not_unique - def radius_of_gyration(group, pbc=False, **kwargs): + @_pbc_to_wrap + def radius_of_gyration(group, wrap=False, **kwargs): """Radius of gyration. Parameters ---------- - pbc : bool, optional + wrap : bool, optional If ``True``, move all atoms within the primary unit cell before calculation. [``False``] - .. versionchanged:: 0.8 Added *pbc* keyword - + .. 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 = group.atoms masses = atomgroup.masses - com = atomgroup.center_of_mass(pbc=pbc) - if pbc: + com = atomgroup.center_of_mass(wrap=wrap) + if wrap: recenteredpos = atomgroup.pack_into_box(inplace=False) - com else: recenteredpos = atomgroup.positions - com @@ -1643,27 +1700,32 @@ def radius_of_gyration(group, pbc=False, **kwargs): ('radius_of_gyration', radius_of_gyration)) @warn_if_not_unique - def shape_parameter(group, pbc=False, **kwargs): + @_pbc_to_wrap + def shape_parameter(group, wrap=False): """Shape parameter. See [Dima2004a]_ for background information. Parameters ---------- - pbc : bool, optional + wrap : bool, optional If ``True``, move all atoms within the primary unit cell before calculation. [``False``] .. versionadded:: 0.7.7 - .. versionchanged:: 0.8 Added *pbc* keyword - + .. 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. + Superfluous kwargs were removed. """ atomgroup = group.atoms masses = atomgroup.masses - com = atomgroup.center_of_mass(pbc=pbc) - if pbc: + com = atomgroup.center_of_mass(wrap=wrap) + if wrap: recenteredpos = atomgroup.pack_into_box(inplace=False) - com else: recenteredpos = atomgroup.positions - com @@ -1683,15 +1745,16 @@ def shape_parameter(group, pbc=False, **kwargs): ('shape_parameter', shape_parameter)) @warn_if_not_unique - @check_pbc_and_unwrap - def asphericity(group, pbc=False, unwrap=None, compound='group'): + @_pbc_to_wrap + @check_wrap_and_unwrap + def asphericity(group, wrap=False, unwrap=None, compound='group'): """Asphericity. See [Dima2004b]_ for background information. Parameters ---------- - pbc : bool, optional + wrap : bool, optional If ``True``, move all atoms within the primary unit cell before calculation. [``False``] unwrap : bool, optional @@ -1701,20 +1764,24 @@ def asphericity(group, pbc=False, unwrap=None, compound='group'): .. versionadded:: 0.7.7 - .. versionchanged:: 0.8 Added *pbc* keyword - .. versionchanged:: 0.20.0 Added *unwrap* and *compound* parameter - + .. versionchanged:: 0.8 + Added `pbc` keyword + .. versionchanged:: 0.20.0 + Added *unwrap* and *compound* parameter + .. 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 = group.atoms masses = atomgroup.masses com = atomgroup.center_of_mass( - pbc=pbc, unwrap=unwrap, compound=compound) + wrap=wrap, unwrap=unwrap, compound=compound) if compound != 'group': com = (com * group.masses[:, None] ).sum(axis=0) / group.masses.sum() - if pbc: + if wrap: recenteredpos = (atomgroup.pack_into_box(inplace=False) - com) elif unwrap: recenteredpos = (atomgroup.unwrap(inplace=False) - com) @@ -1737,7 +1804,8 @@ def asphericity(group, pbc=False, unwrap=None, compound='group'): ('asphericity', asphericity)) @warn_if_not_unique - def principal_axes(group, pbc=False): + @_pbc_to_wrap + def principal_axes(group, wrap=False): """Calculate the principal axes from the moment of inertia. e1,e2,e3 = AtomGroup.principal_axes() @@ -1750,7 +1818,7 @@ def principal_axes(group, pbc=False): Parameters ---------- - pbc : bool, optional + wrap : bool, optional If ``True``, move all atoms within the primary unit cell before calculation. [``False``] @@ -1761,13 +1829,16 @@ def principal_axes(group, pbc=False): ``v[2]`` as third eigenvector. - .. versionchanged:: 0.8 Added *pbc* keyword + .. versionchanged:: 0.8 + Added `pbc` keyword .. versionchanged:: 1.0.0 - Always return principal axes in right-hand convention. - + Always return principal axes in right-hand convention. + .. 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 = group.atoms - e_val, e_vec = np.linalg.eig(atomgroup.moment_of_inertia(pbc=pbc)) + e_val, e_vec = np.linalg.eig(atomgroup.moment_of_inertia(wrap=wrap)) # Sort indices = np.argsort(e_val)[::-1] diff --git a/package/MDAnalysis/transformations/rotate.py b/package/MDAnalysis/transformations/rotate.py index 0c9bd80fc96..ad78a632945 100644 --- a/package/MDAnalysis/transformations/rotate.py +++ b/package/MDAnalysis/transformations/rotate.py @@ -173,7 +173,7 @@ def __init__(self, raise TypeError(errmsg) from None self.center_method = partial(self.atoms.center, self.weights, - pbc=self.wrap) + wrap=self.wrap) else: raise ValueError('A point or an AtomGroup must be specified') diff --git a/package/MDAnalysis/transformations/translate.py b/package/MDAnalysis/transformations/translate.py index ac9d3b1ffd2..72ccebb474d 100644 --- a/package/MDAnalysis/transformations/translate.py +++ b/package/MDAnalysis/transformations/translate.py @@ -148,10 +148,10 @@ def __init__(self, ag, center='geometry', point=None, wrap=False, try: if self.center == 'geometry': self.center_method = partial(self.ag.center_of_geometry, - pbc=pbc_arg) + wrap=pbc_arg) elif self.center == 'mass': self.center_method = partial(self.ag.center_of_mass, - pbc=pbc_arg) + wrap=pbc_arg) else: raise ValueError(f'{self.center} is valid for center') except AttributeError: diff --git a/testsuite/MDAnalysisTests/core/test_atomgroup.py b/testsuite/MDAnalysisTests/core/test_atomgroup.py index aa71863860a..281ba6c385c 100644 --- a/testsuite/MDAnalysisTests/core/test_atomgroup.py +++ b/testsuite/MDAnalysisTests/core/test_atomgroup.py @@ -479,19 +479,19 @@ def test_center_unwrap(self, level, compound, is_triclinic): group = group.segments # get the expected results - center = group.center(weights=None, pbc=False, + center = group.center(weights=None, wrap=False, compound=compound, unwrap=True) ref_center = u.center(compound=compound) assert_almost_equal(ref_center, center, decimal=4) - def test_center_unwrap_pbc_true_group(self): + def test_center_unwrap_wrap_true_group(self): u = UnWrapUniverse(is_triclinic=False) # select group appropriate for compound: group = u.atoms[39:47] # molecule 12 with pytest.raises(ValueError): group.center(weights=None, compound='group', - unwrap=True, pbc=True) + unwrap=True, wrap=True) class TestSplit(object): @@ -1131,7 +1131,7 @@ def ag(self): universe = mda.Universe(TRZ_psf, TRZ) return universe.residues[0:3] - @pytest.mark.parametrize('pbc, ref', ((True, ref_PBC), + @pytest.mark.parametrize('wrap, ref', ((True, ref_PBC), (False, ref_noPBC))) @pytest.mark.parametrize('method_name', ('center_of_geometry', 'center_of_mass', @@ -1142,12 +1142,12 @@ def ag(self): 'bbox', 'bsphere', 'principal_axes')) - def test_pbc(self, ag, pbc, ref, method_name): + def test_wrap(self, ag, wrap, ref, method_name): method = getattr(ag, method_name) - if pbc: - result = method(pbc=True) + if wrap: + result = method(wrap=True) else: - # Test no-pbc as the default behaviour + # Test no-wrap as the default behaviour result = method() if method_name == 'bsphere': @@ -1266,7 +1266,7 @@ def test_center_duplicates(self, ag, method_name): ('segids', 'segments'))) def test_center_compounds(self, ag, name, compound, method_name): ref = [getattr(a, method_name)() for a in ag.groupby(name).values()] - vals = getattr(ag, method_name)(pbc=False, compound=compound) + vals = getattr(ag, method_name)(wrap=False, compound=compound) assert_almost_equal(vals, ref, decimal=5) @pytest.mark.parametrize('method_name', ('center_of_geometry', @@ -1279,9 +1279,7 @@ def test_center_compounds_pbc(self, ag, name, compound, ag.dimensions = [50, 50, 50, 90, 90, 90] ref = [getattr(a, method_name)(unwrap=unwrap) for a in ag.groupby(name).values()] - ref = distances.apply_PBC(np.asarray(ref, dtype=np.float32), - ag.dimensions) - vals = getattr(ag, method_name)(pbc=True, compound=compound, + vals = getattr(ag, method_name)(compound=compound, unwrap=unwrap) assert_almost_equal(vals, ref, decimal=5) @@ -1293,7 +1291,7 @@ def test_center_compounds_special(self, ag_molfrg, name, compound, method_name): ref = [getattr(a, method_name)() for a in ag_molfrg.groupby(name).values()] - vals = getattr(ag_molfrg, method_name)(pbc=False, compound=compound) + vals = getattr(ag_molfrg, method_name)(wrap=False, compound=compound) assert_almost_equal(vals, ref, decimal=5) @pytest.mark.parametrize('method_name', ('center_of_geometry', @@ -1306,9 +1304,7 @@ def test_center_compounds_special_pbc(self, ag_molfrg, name, compound, ag_molfrg.dimensions = [50, 50, 50, 90, 90, 90] ref = [getattr(a, method_name)(unwrap=unwrap) for a in ag_molfrg.groupby(name).values()] - ref = distances.apply_PBC(np.asarray(ref, dtype=np.float32), - ag_molfrg.dimensions) - vals = getattr(ag_molfrg, method_name)(pbc=True, compound=compound, + vals = getattr(ag_molfrg, method_name)(compound=compound, unwrap=unwrap) assert_almost_equal(vals, ref, decimal=5) @@ -1326,11 +1322,11 @@ def test_center_compounds_special_fail(self, ag_no_molfrg, compound): np.array([2.0]))) @pytest.mark.parametrize('compound', ('group', 'residues', 'segments', 'molecules', 'fragments')) - @pytest.mark.parametrize('pbc', (False, True)) - def test_center_compounds_single(self, ag_molfrg, pbc, weights, compound): + @pytest.mark.parametrize('wrap', (False, True)) + def test_center_compounds_single(self, ag_molfrg, wrap, weights, compound): at = ag_molfrg[0] if weights is None or weights[0] != 0.0: - if pbc: + if wrap: ref = distances.apply_PBC(at.position, ag_molfrg.dimensions) ref = ref.astype(np.float64) else: @@ -1340,24 +1336,24 @@ def test_center_compounds_single(self, ag_molfrg, pbc, weights, compound): if compound != 'group': ref = ref.reshape((1, 3)) ag_s = mda.AtomGroup([at]) - assert_equal(ref, ag_s.center(weights, pbc=pbc, compound=compound)) + assert_equal(ref, ag_s.center(weights, wrap=wrap, compound=compound)) - @pytest.mark.parametrize('pbc', (False, True)) + @pytest.mark.parametrize('wrap', (False, True)) @pytest.mark.parametrize('weights', (None, np.array([]))) @pytest.mark.parametrize('compound', ('group', 'residues', 'segments', 'molecules', 'fragments')) - def test_center_compounds_empty(self, ag_molfrg, pbc, weights, compound): + def test_center_compounds_empty(self, ag_molfrg, wrap, weights, compound): ref = np.empty((0, 3), dtype=np.float64) ag_e = mda.AtomGroup([], ag_molfrg.universe) - assert_equal(ref, ag_e.center(weights, pbc=pbc, compound=compound)) + assert_equal(ref, ag_e.center(weights, wrap=wrap, compound=compound)) - @pytest.mark.parametrize('pbc', (False, True)) + @pytest.mark.parametrize('wrap', (False, True)) @pytest.mark.parametrize('name, compound', (('', 'group'), ('resids', 'residues'), ('segids', 'segments'), ('molnums', 'molecules'), ('fragindices', 'fragments'))) - def test_center_compounds_zero_weights(self, ag_molfrg, pbc, name, + def test_center_compounds_zero_weights(self, ag_molfrg, wrap, name, compound): if compound == 'group': ref = np.full((3,), np.nan) @@ -1365,7 +1361,7 @@ def test_center_compounds_zero_weights(self, ag_molfrg, pbc, name, n_compounds = len(ag_molfrg.groupby(name)) ref = np.full((n_compounds, 3), np.nan, dtype=np.float64) weights = np.zeros(len(ag_molfrg)) - assert_equal(ref, ag_molfrg.center(weights, pbc=pbc, + assert_equal(ref, ag_molfrg.center(weights, wrap=wrap, compound=compound)) def test_coordinates(self, ag): diff --git a/testsuite/MDAnalysisTests/core/test_groups.py b/testsuite/MDAnalysisTests/core/test_groups.py index 59b7d38d9ef..1c5d0eb3fdc 100644 --- a/testsuite/MDAnalysisTests/core/test_groups.py +++ b/testsuite/MDAnalysisTests/core/test_groups.py @@ -1528,21 +1528,43 @@ def test_VE_no_uni_2(self, components): class TestDecorator(object): - @groups.check_pbc_and_unwrap - def dummy_funtion(cls, compound="group", pbc=True, unwrap=True): + @groups._pbc_to_wrap + @groups.check_wrap_and_unwrap + def dummy_funtion(cls, compound="group", wrap=True, unwrap=True): return 0 @pytest.mark.parametrize('compound', ('fragments', 'molecules', 'residues', 'group', 'segments')) @pytest.mark.parametrize('pbc', (True, False)) @pytest.mark.parametrize('unwrap', (True, False)) - def test_decorator(self, compound, pbc, unwrap): + def test_wrap_and_unwrap_deprecation(self, compound, pbc, unwrap): - if compound == 'group' and pbc and unwrap: + if pbc and unwrap: with pytest.raises(ValueError): + # We call a deprecated argument that does not appear in the + # function's signature. This is done on purpose to test the + # deprecation. We need to tell the linter. + # pylint: disable-next=unexpected-keyword-arg self.dummy_funtion(compound=compound, pbc=pbc, unwrap=unwrap) else: - assert_equal(self.dummy_funtion(compound=compound, pbc=pbc, unwrap=unwrap), 0) + with pytest.warns(DeprecationWarning): + # We call a deprecated argument that does not appear in the + # function's signature. This is done on purpose to test the + # deprecation. We need to tell the linter. + # pylint: disable-next=unexpected-keyword-arg + assert self.dummy_funtion(compound=compound, pbc=pbc, unwrap=unwrap) == 0 + + @pytest.mark.parametrize('compound', ('fragments', 'molecules', 'residues', + 'group', 'segments')) + @pytest.mark.parametrize('wrap', (True, False)) + @pytest.mark.parametrize('unwrap', (True, False)) + def test_wrap_and_unwrap(self, compound, wrap, unwrap): + + if wrap and unwrap: + with pytest.raises(ValueError): + self.dummy_funtion(compound=compound, wrap=wrap, unwrap=unwrap) + else: + assert self.dummy_funtion(compound=compound, wrap=wrap, unwrap=unwrap) == 0 @pytest.fixture() diff --git a/testsuite/MDAnalysisTests/core/test_wrap.py b/testsuite/MDAnalysisTests/core/test_wrap.py index b8ad82a7778..ecbe61fcee5 100644 --- a/testsuite/MDAnalysisTests/core/test_wrap.py +++ b/testsuite/MDAnalysisTests/core/test_wrap.py @@ -74,10 +74,10 @@ def test_wrap_pass(self, level, compound, center, is_triclinic): if compound == 'atoms': assert_in_box(group.atoms.positions, group.dimensions) elif center == 'com': - compos = group.atoms.center_of_mass(pbc=False, compound=compound) + compos = group.atoms.center_of_mass(wrap=False, compound=compound) assert_in_box(compos, group.dimensions) else: - cogpos = group.atoms.center_of_geometry(pbc=False, + cogpos = group.atoms.center_of_geometry(wrap=False, compound=compound) assert_in_box(cogpos, group.dimensions) @@ -441,9 +441,9 @@ def test_wrap_group(self, u, center): ag = u.atoms[:100] ag.wrap(compound='group', center=center) if center == 'com': - ctrpos = ag.center_of_mass(pbc=False, compound='group') + ctrpos = ag.center_of_mass(wrap=False, compound='group') elif center == 'cog': - ctrpos = ag.center_of_geometry(pbc=False, compound='group') + ctrpos = ag.center_of_geometry(wrap=False, compound='group') assert_in_box(ctrpos, u.dimensions) @pytest.mark.parametrize('center', ('com', 'cog')) @@ -451,9 +451,9 @@ def test_wrap_residues(self, u, center): ag = u.atoms[300:400].residues ag.wrap(compound='residues', center=center) if center == 'com': - ctrpos = ag.center_of_mass(pbc=False, compound='residues') + ctrpos = ag.center_of_mass(wrap=False, compound='residues') elif center == 'cog': - ctrpos = ag.center_of_geometry(pbc=False, compound='residues') + ctrpos = ag.center_of_geometry(wrap=False, compound='residues') assert_in_box(ctrpos, u.dimensions) @pytest.mark.parametrize('center', ('com', 'cog')) @@ -461,9 +461,9 @@ def test_wrap_segments(self, u, center): ag = u.atoms[1000:1200] ag.wrap(compound='segments', center=center) if center == 'com': - ctrpos = ag.center_of_mass(pbc=False, compound='segments') + ctrpos = ag.center_of_mass(wrap=False, compound='segments') elif center == 'cog': - ctrpos = ag.center_of_geometry(pbc=False, compound='segments') + ctrpos = ag.center_of_geometry(wrap=False, compound='segments') assert_in_box(ctrpos, u.dimensions) @pytest.mark.parametrize('center', ('com', 'cog')) @@ -471,7 +471,7 @@ def test_wrap_fragments(self, u, center): ag = u.atoms[:250] ag.wrap(compound='fragments', center=center) if center == 'com': - ctrpos = ag.center_of_mass(pbc=False, compound='fragments') + ctrpos = ag.center_of_mass(wrap=False, compound='fragments') elif center == 'cog': - ctrpos = ag.center_of_geometry(pbc=False, compound='fragments') + ctrpos = ag.center_of_geometry(wrap=False, compound='fragments') assert_in_box(ctrpos, u.dimensions)