Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for issue #4655 Removing mutable data structures and function calls as default arguments in the entire codebase #4810

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion package/CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ The rules for this file:


-------------------------------------------------------------------------------
??/??/?? IAlibay
??/??/?? IAlibay, tanishy7777

* 2.9.0

Fixes
* Replaced mutable defaults with None and initialized them within
the function to prevent shared state.

Enhancements

Expand Down
4 changes: 3 additions & 1 deletion package/MDAnalysis/analysis/align.py
Original file line number Diff line number Diff line change
Expand Up @@ -1592,7 +1592,9 @@ def get_matching_atoms(ag1, ag2, tol_mass=0.1, strict=False, match_atoms=True):
rsize_mismatches = np.absolute(rsize1 - rsize2)
mismatch_mask = (rsize_mismatches > 0)
if np.any(mismatch_mask):
def get_atoms_byres(g, match_mask=np.logical_not(mismatch_mask)):
def get_atoms_byres(g, match_mask=None):
if match_mask is None:
match_mask = np.logical_not(mismatch_mask)
# not pretty... but need to do things on a per-atom basis in
# order to preserve original selection
ag = g.atoms
Expand Down
4 changes: 3 additions & 1 deletion package/MDAnalysis/analysis/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@

def _compute(self, indexed_frames: np.ndarray,
verbose: bool = None,
*, progressbar_kwargs={}) -> "AnalysisBase":
*, progressbar_kwargs=None) -> "AnalysisBase":
"""Perform the calculation on a balanced slice of frames
that have been setup prior to that using _setup_computation_groups()

Expand All @@ -500,6 +500,8 @@

.. versionadded:: 2.8.0
"""
if progressbar_kwargs is None:
progressbar_kwargs = {}

Check warning on line 504 in package/MDAnalysis/analysis/base.py

View check run for this annotation

Codecov / codecov/patch

package/MDAnalysis/analysis/base.py#L504

Added line #L504 was not covered by tests
logger.info("Choosing frames to analyze")
# if verbose unchanged, use class default
verbose = getattr(self, "_verbose", False) if verbose is None else verbose
Expand Down
5 changes: 3 additions & 2 deletions package/MDAnalysis/analysis/encore/clustering/cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@


def cluster(ensembles,
method = ClusteringMethod.AffinityPropagationNative(),
method=None,
select="name CA",
distance_matrix=None,
allow_collapsed_result=True,
Expand Down Expand Up @@ -154,7 +154,8 @@ def cluster(ensembles,
[array([1, 1, 1, 1, 2]), array([1, 1, 1, 1, 1])]

"""

if method is None:
method = ClusteringMethod.AffinityPropagationNative()
# Internally, ensembles are always transformed to a list of lists
if ensembles is not None:
if not hasattr(ensembles, '__iter__'):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@


def reduce_dimensionality(ensembles,
method=StochasticProximityEmbeddingNative(),
method=None,
select="name CA",
distance_matrix=None,
allow_collapsed_result=True,
Expand Down Expand Up @@ -152,7 +152,8 @@ def reduce_dimensionality(ensembles,
encore.StochasticProximityEmbeddingNative(dimension=2)])

"""

if method is None:
method = StochasticProximityEmbeddingNative()
if ensembles is not None:
if not hasattr(ensembles, '__iter__'):
ensembles = [ensembles]
Expand Down
67 changes: 35 additions & 32 deletions package/MDAnalysis/analysis/encore/similarity.py
Original file line number Diff line number Diff line change
Expand Up @@ -954,12 +954,7 @@ def hes(ensembles,

def ces(ensembles,
select="name CA",
clustering_method=AffinityPropagationNative(
preference=-1.0,
max_iter=500,
convergence_iter=50,
damping=0.9,
add_noise=True),
clustering_method=None,
distance_matrix=None,
estimate_error=False,
bootstrapping_samples=10,
Expand Down Expand Up @@ -1084,7 +1079,13 @@ def ces(ensembles,
[0.25331629 0. ]]

"""

if clustering_method is None:
clustering_method = AffinityPropagationNative(
preference=-1.0,
max_iter=500,
convergence_iter=50,
damping=0.9,
add_noise=True)
for ensemble in ensembles:
ensemble.transfer_to_memory()

Expand Down Expand Up @@ -1220,13 +1221,7 @@ def ces(ensembles,

def dres(ensembles,
select="name CA",
dimensionality_reduction_method = StochasticProximityEmbeddingNative(
dimension=3,
distance_cutoff = 1.5,
min_lam=0.1,
max_lam=2.0,
ncycle=100,
nstep=10000),
dimensionality_reduction_method=None,
distance_matrix=None,
nsamples=1000,
estimate_error=False,
Expand Down Expand Up @@ -1354,7 +1349,14 @@ def dres(ensembles,
:mod:`MDAnalysis.analysis.encore.dimensionality_reduction.reduce_dimensionality``

"""

if dimensionality_reduction_method is None:
dimensionality_reduction_method = StochasticProximityEmbeddingNative(
dimension=3,
distance_cutoff=1.5,
min_lam=0.1,
max_lam=2.0,
ncycle=100,
nstep=10000)
for ensemble in ensembles:
ensemble.transfer_to_memory()

Expand Down Expand Up @@ -1487,12 +1489,7 @@ def dres(ensembles,
def ces_convergence(original_ensemble,
window_size,
select="name CA",
clustering_method=AffinityPropagationNative(
preference=-1.0,
max_iter=500,
convergence_iter=50,
damping=0.9,
add_noise=True),
clustering_method=None,
ncores=1):
"""
Use the CES to evaluate the convergence of the ensemble/trajectory.
Expand Down Expand Up @@ -1559,7 +1556,13 @@ def ces_convergence(original_ensemble,
[0. ]]

"""

if clustering_method is None:
clustering_method = AffinityPropagationNative(
preference=-1.0,
max_iter=500,
convergence_iter=50,
damping=0.9,
add_noise=True)
ensembles = prepare_ensembles_for_convergence_increasing_window(
original_ensemble, window_size, select=select)

Expand Down Expand Up @@ -1587,15 +1590,7 @@ def ces_convergence(original_ensemble,
def dres_convergence(original_ensemble,
window_size,
select="name CA",
dimensionality_reduction_method = \
StochasticProximityEmbeddingNative(
dimension=3,
distance_cutoff=1.5,
min_lam=0.1,
max_lam=2.0,
ncycle=100,
nstep=10000
),
dimensionality_reduction_method=None,
nsamples=1000,
ncores=1):
"""
Expand Down Expand Up @@ -1660,7 +1655,15 @@ def dres_convergence(original_ensemble,
much the trajectory keeps on resampling the same ares of the conformational
space, and therefore of convergence.
"""

if dimensionality_reduction_method is None:
dimensionality_reduction_method = StochasticProximityEmbeddingNative(
dimension=3,
distance_cutoff=1.5,
min_lam=0.1,
max_lam=2.0,
ncycle=100,
nstep=10000
)
ensembles = prepare_ensembles_for_convergence_increasing_window(
original_ensemble, window_size, select=select)

Expand Down
9 changes: 6 additions & 3 deletions package/MDAnalysis/analysis/helix_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@
return np.rad2deg(to_ortho)


def helix_analysis(positions, ref_axis=[0, 0, 1]):
def helix_analysis(positions, ref_axis=None):
r"""
Calculate helix properties from atomic coordinates.

Expand Down Expand Up @@ -244,7 +244,8 @@
# θ: local_twists
# origin: origins
# local_axes: perpendicular to plane of screen. Orthogonal to "bisectors"

if ref_axis is None:
ref_axis = (0, 0, 1)

Check warning on line 248 in package/MDAnalysis/analysis/helix_analysis.py

View check run for this annotation

Codecov / codecov/patch

package/MDAnalysis/analysis/helix_analysis.py#L248

Added line #L248 was not covered by tests
vectors = positions[1:] - positions[:-1] # (n_res-1, 3)
bisectors = vectors[:-1] - vectors[1:] # (n_res-2, 3)
bimags = mdamath.pnorm(bisectors) # (n_res-2,)
Expand Down Expand Up @@ -387,11 +388,13 @@
'local_screw_angles': (-2,),
}

def __init__(self, universe, select='name CA', ref_axis=[0, 0, 1],
def __init__(self, universe, select='name CA', ref_axis=None,
tanishy7777 marked this conversation as resolved.
Show resolved Hide resolved
verbose=False, flatten_single_helix=True,
split_residue_sequences=True):
super(HELANAL, self).__init__(universe.universe.trajectory,
verbose=verbose)
if ref_axis is None:
ref_axis = (0, 0, 1)
selections = util.asiterable(select)
atomgroups = [universe.select_atoms(s) for s in selections]
consecutive = []
Expand Down
4 changes: 3 additions & 1 deletion package/MDAnalysis/core/universe.py
Original file line number Diff line number Diff line change
Expand Up @@ -1476,7 +1476,7 @@
@classmethod
def from_smiles(cls, smiles, sanitize=True, addHs=True,
generate_coordinates=True, numConfs=1,
rdkit_kwargs={}, **kwargs):
rdkit_kwargs=None, **kwargs):
"""Create a Universe from a SMILES string with rdkit

Parameters
Expand Down Expand Up @@ -1530,6 +1530,8 @@
.. versionadded:: 2.0.0

"""
if rdkit_kwargs is None:
rdkit_kwargs = {}

Check warning on line 1534 in package/MDAnalysis/core/universe.py

View check run for this annotation

Codecov / codecov/patch

package/MDAnalysis/core/universe.py#L1534

Added line #L1534 was not covered by tests
try:
from rdkit import Chem
from rdkit.Chem import AllChem
Expand Down
4 changes: 3 additions & 1 deletion package/MDAnalysis/topology/ITPParser.py
Original file line number Diff line number Diff line change
Expand Up @@ -429,8 +429,10 @@

return atom_order, new_params, molnums, self.moltypes, residx

def add_param(self, line, container, n_funct=2, funct_values=[]):
def add_param(self, line, container, n_funct=2, funct_values=None):
"""Add defined GROMACS directive lines, only if the funct is in ``funct_values``"""
if funct_values is None:
funct_values = []

Check warning on line 435 in package/MDAnalysis/topology/ITPParser.py

View check run for this annotation

Codecov / codecov/patch

package/MDAnalysis/topology/ITPParser.py#L435

Added line #L435 was not covered by tests
values = line.split()
funct = int(values[n_funct])
if funct in funct_values:
Expand Down
Loading