From b3c52197fa6e37ee9228796e647571767b15e2a7 Mon Sep 17 00:00:00 2001 From: Tanish Yelgoe <23110328@iitgn.ac.in> Date: Sat, 30 Nov 2024 16:21:52 +0530 Subject: [PATCH 1/5] Fix for issue 4655 removing mutable data structures and function calls as default arguments in the entire codebase --- package/MDAnalysis/analysis/align.py | 4 +- package/MDAnalysis/analysis/base.py | 4 +- .../analysis/encore/clustering/cluster.py | 5 +- .../reduce_dimensionality.py | 5 +- .../MDAnalysis/analysis/encore/similarity.py | 70 ++++++++++--------- package/MDAnalysis/analysis/helix_analysis.py | 9 ++- package/MDAnalysis/core/universe.py | 4 +- package/MDAnalysis/topology/ITPParser.py | 4 +- 8 files changed, 61 insertions(+), 44 deletions(-) diff --git a/package/MDAnalysis/analysis/align.py b/package/MDAnalysis/analysis/align.py index fd7f15a822..d6b41340d5 100644 --- a/package/MDAnalysis/analysis/align.py +++ b/package/MDAnalysis/analysis/align.py @@ -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 diff --git a/package/MDAnalysis/analysis/base.py b/package/MDAnalysis/analysis/base.py index f940af58e8..4e7f58dc0b 100644 --- a/package/MDAnalysis/analysis/base.py +++ b/package/MDAnalysis/analysis/base.py @@ -479,7 +479,7 @@ def _conclude(self): 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() @@ -500,6 +500,8 @@ def _compute(self, indexed_frames: np.ndarray, .. versionadded:: 2.8.0 """ + if progressbar_kwargs is None: + progressbar_kwargs = {} logger.info("Choosing frames to analyze") # if verbose unchanged, use class default verbose = getattr(self, "_verbose", False) if verbose is None else verbose diff --git a/package/MDAnalysis/analysis/encore/clustering/cluster.py b/package/MDAnalysis/analysis/encore/clustering/cluster.py index 1c43f2cfd7..8655de248b 100644 --- a/package/MDAnalysis/analysis/encore/clustering/cluster.py +++ b/package/MDAnalysis/analysis/encore/clustering/cluster.py @@ -45,7 +45,7 @@ def cluster(ensembles, - method = ClusteringMethod.AffinityPropagationNative(), + method = None, select="name CA", distance_matrix=None, allow_collapsed_result=True, @@ -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__'): diff --git a/package/MDAnalysis/analysis/encore/dimensionality_reduction/reduce_dimensionality.py b/package/MDAnalysis/analysis/encore/dimensionality_reduction/reduce_dimensionality.py index 281d681203..dbf14b7443 100644 --- a/package/MDAnalysis/analysis/encore/dimensionality_reduction/reduce_dimensionality.py +++ b/package/MDAnalysis/analysis/encore/dimensionality_reduction/reduce_dimensionality.py @@ -45,7 +45,7 @@ def reduce_dimensionality(ensembles, - method=StochasticProximityEmbeddingNative(), + method=None, select="name CA", distance_matrix=None, allow_collapsed_result=True, @@ -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] diff --git a/package/MDAnalysis/analysis/encore/similarity.py b/package/MDAnalysis/analysis/encore/similarity.py index 4fe6f0e35a..b504974c48 100644 --- a/package/MDAnalysis/analysis/encore/similarity.py +++ b/package/MDAnalysis/analysis/encore/similarity.py @@ -178,6 +178,7 @@ import MDAnalysis as mda +from . import dimensionality_reduction from ...coordinates.memory import MemoryReader from .confdistmatrix import get_distance_matrix from .bootstrap import (get_distance_matrix_bootstrap_samples, @@ -185,7 +186,7 @@ from .clustering.cluster import cluster from .clustering.ClusteringMethod import AffinityPropagationNative from .dimensionality_reduction.DimensionalityReductionMethod import ( - StochasticProximityEmbeddingNative) + StochasticProximityEmbeddingNative, DimensionalityReductionMethod) from .dimensionality_reduction.reduce_dimensionality import ( reduce_dimensionality) from .covariance import ( @@ -954,12 +955,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, @@ -1084,7 +1080,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() @@ -1220,13 +1222,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, @@ -1354,7 +1350,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() @@ -1487,12 +1490,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. @@ -1559,7 +1557,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) @@ -1587,15 +1591,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): """ @@ -1660,7 +1656,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) diff --git a/package/MDAnalysis/analysis/helix_analysis.py b/package/MDAnalysis/analysis/helix_analysis.py index 9c287fb750..f5fc13d407 100644 --- a/package/MDAnalysis/analysis/helix_analysis.py +++ b/package/MDAnalysis/analysis/helix_analysis.py @@ -186,7 +186,7 @@ def local_screw_angles(global_axis, ref_axis, helix_directions): 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. @@ -244,7 +244,8 @@ def helix_analysis(positions, ref_axis=[0, 0, 1]): # θ: local_twists # origin: origins # local_axes: perpendicular to plane of screen. Orthogonal to "bisectors" - + if ref_axis is None: + ref_axis = [0, 0, 1] 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,) @@ -387,11 +388,13 @@ class HELANAL(AnalysisBase): '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, 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 = [] diff --git a/package/MDAnalysis/core/universe.py b/package/MDAnalysis/core/universe.py index 7fed2cde8c..61ea3eb2b3 100644 --- a/package/MDAnalysis/core/universe.py +++ b/package/MDAnalysis/core/universe.py @@ -1476,7 +1476,7 @@ def _fragdict(self): @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 @@ -1530,6 +1530,8 @@ def from_smiles(cls, smiles, sanitize=True, addHs=True, .. versionadded:: 2.0.0 """ + if rdkit_kwargs is None: + rdkit_kwargs = {} try: from rdkit import Chem from rdkit.Chem import AllChem diff --git a/package/MDAnalysis/topology/ITPParser.py b/package/MDAnalysis/topology/ITPParser.py index 83b43711c8..9968f854df 100644 --- a/package/MDAnalysis/topology/ITPParser.py +++ b/package/MDAnalysis/topology/ITPParser.py @@ -429,8 +429,10 @@ def shift_indices(self, atomid=0, resid=0, molnum=0, cgnr=0, n_res=0, n_atoms=0) 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 = [] values = line.split() funct = int(values[n_funct]) if funct in funct_values: From b1eb89fec20ab2ff6311ed065fd74826c5f1eb99 Mon Sep 17 00:00:00 2001 From: Tanish Yelgoe <23110328@iitgn.ac.in> Date: Sat, 30 Nov 2024 16:30:12 +0530 Subject: [PATCH 2/5] formatting as per PEP8 and updated CHANGELOG --- package/CHANGELOG | 8 +++++++- package/MDAnalysis/analysis/encore/clustering/cluster.py | 2 +- package/MDAnalysis/analysis/encore/similarity.py | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index 5bce69eaec..d697f3f9d0 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -14,11 +14,17 @@ The rules for this file: ------------------------------------------------------------------------------- -??/??/?? IAlibay +11/30/24 IAlibay, tanishy7777 * 2.9.0 Fixes + * Addressed linting issues by resolving instances of mutable default + arguments (B006) and function calls as default arguments (B008). + Replaced mutable defaults with None and initialized them within + the function to prevent shared state. Moved function calls from + argument defaults into the function body to ensure proper execution + context and avoid unintended behavior. Enhancements diff --git a/package/MDAnalysis/analysis/encore/clustering/cluster.py b/package/MDAnalysis/analysis/encore/clustering/cluster.py index 8655de248b..8b13c8d492 100644 --- a/package/MDAnalysis/analysis/encore/clustering/cluster.py +++ b/package/MDAnalysis/analysis/encore/clustering/cluster.py @@ -45,7 +45,7 @@ def cluster(ensembles, - method = None, + method=None, select="name CA", distance_matrix=None, allow_collapsed_result=True, diff --git a/package/MDAnalysis/analysis/encore/similarity.py b/package/MDAnalysis/analysis/encore/similarity.py index b504974c48..2a70d755e9 100644 --- a/package/MDAnalysis/analysis/encore/similarity.py +++ b/package/MDAnalysis/analysis/encore/similarity.py @@ -1353,7 +1353,7 @@ def dres(ensembles, if dimensionality_reduction_method is None: dimensionality_reduction_method = StochasticProximityEmbeddingNative( dimension=3, - distance_cutoff = 1.5, + distance_cutoff=1.5, min_lam=0.1, max_lam=2.0, ncycle=100, From c36727330ee111c6527fd30d2b5c1f720cc111a1 Mon Sep 17 00:00:00 2001 From: Tanish Yelgoe <23110328@iitgn.ac.in> Date: Sat, 30 Nov 2024 16:36:52 +0530 Subject: [PATCH 3/5] removing unused imports which were added accidentally --- package/MDAnalysis/analysis/encore/similarity.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/package/MDAnalysis/analysis/encore/similarity.py b/package/MDAnalysis/analysis/encore/similarity.py index 2a70d755e9..87c9c47bcb 100644 --- a/package/MDAnalysis/analysis/encore/similarity.py +++ b/package/MDAnalysis/analysis/encore/similarity.py @@ -178,7 +178,6 @@ import MDAnalysis as mda -from . import dimensionality_reduction from ...coordinates.memory import MemoryReader from .confdistmatrix import get_distance_matrix from .bootstrap import (get_distance_matrix_bootstrap_samples, @@ -186,7 +185,7 @@ from .clustering.cluster import cluster from .clustering.ClusteringMethod import AffinityPropagationNative from .dimensionality_reduction.DimensionalityReductionMethod import ( - StochasticProximityEmbeddingNative, DimensionalityReductionMethod) + StochasticProximityEmbeddingNative) from .dimensionality_reduction.reduce_dimensionality import ( reduce_dimensionality) from .covariance import ( From 48bfd5d0df0396d9774a576b01ed57d69d6725a5 Mon Sep 17 00:00:00 2001 From: Tanish Yelgoe <143334319+tanishy7777@users.noreply.github.com> Date: Sun, 1 Dec 2024 13:46:38 +0530 Subject: [PATCH 4/5] Update CHANGELOG shortened the fixes in the changelog --- package/CHANGELOG | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index d697f3f9d0..7c0f62190f 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -14,17 +14,13 @@ The rules for this file: ------------------------------------------------------------------------------- -11/30/24 IAlibay, tanishy7777 +??/??/?? IAlibay, tanishy7777 * 2.9.0 Fixes - * Addressed linting issues by resolving instances of mutable default - arguments (B006) and function calls as default arguments (B008). - Replaced mutable defaults with None and initialized them within - the function to prevent shared state. Moved function calls from - argument defaults into the function body to ensure proper execution - context and avoid unintended behavior. + * Replaced mutable defaults with None and initialized them within + the function to prevent shared state. Enhancements From dc712ac15c19e14e2821611b46559735449169be Mon Sep 17 00:00:00 2001 From: Tanish Yelgoe <143334319+tanishy7777@users.noreply.github.com> Date: Sun, 1 Dec 2024 13:53:14 +0530 Subject: [PATCH 5/5] Update helix_analysis.py using tuples instead of list for ref_axis --- package/MDAnalysis/analysis/helix_analysis.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package/MDAnalysis/analysis/helix_analysis.py b/package/MDAnalysis/analysis/helix_analysis.py index f5fc13d407..399c441bee 100644 --- a/package/MDAnalysis/analysis/helix_analysis.py +++ b/package/MDAnalysis/analysis/helix_analysis.py @@ -245,7 +245,7 @@ def helix_analysis(positions, ref_axis=None): # origin: origins # local_axes: perpendicular to plane of screen. Orthogonal to "bisectors" if ref_axis is None: - ref_axis = [0, 0, 1] + ref_axis = (0, 0, 1) 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,) @@ -394,7 +394,7 @@ def __init__(self, universe, select='name CA', ref_axis=None, super(HELANAL, self).__init__(universe.universe.trajectory, verbose=verbose) if ref_axis is None: - ref_axis = [0, 0, 1] + ref_axis = (0, 0, 1) selections = util.asiterable(select) atomgroups = [universe.select_atoms(s) for s in selections] consecutive = []