From 3f8c5a6b77898f7cf3154b5720ccf25db79a17d3 Mon Sep 17 00:00:00 2001 From: Irfan Alibay Date: Wed, 2 Dec 2020 18:56:39 +0000 Subject: [PATCH 01/20] Adds basic GH Actions CI workflow (#3040) Fixes #3036 ## Work done in this PR - Adds a continuous integration workflow based on github actions. - Disables previous continuous integration workflow based on TravisCI. - Fixes minor Visibledeprecation warnings with the hole2 tests. --- .travis.yml => .disabled-travis.yml | 0 .github/workflows/gh-ci.yaml | 281 ++++++++++++++++++ maintainer/install_hole.sh | 115 ------- .../MDAnalysisTests/analysis/test_hole2.py | 6 +- 4 files changed, 284 insertions(+), 118 deletions(-) rename .travis.yml => .disabled-travis.yml (100%) create mode 100644 .github/workflows/gh-ci.yaml delete mode 100755 maintainer/install_hole.sh diff --git a/.travis.yml b/.disabled-travis.yml similarity index 100% rename from .travis.yml rename to .disabled-travis.yml diff --git a/.github/workflows/gh-ci.yaml b/.github/workflows/gh-ci.yaml new file mode 100644 index 00000000000..a9fee810031 --- /dev/null +++ b/.github/workflows/gh-ci.yaml @@ -0,0 +1,281 @@ +name: mda_gh_ci +on: + push: + branches: + - develop + - master + pull_request: + branches: + - develop + - master + +defaults: + run: + shell: bash -l {0} + +env: + MDA_CONDA_MIN_DEPS: "pip pytest mmtf-python biopython networkx cython matplotlib scipy griddataformats hypothesis gsd codecov" + MDA_CONDA_EXTRA_DEPS: "seaborn>=0.7.0 clustalw=2.1 netcdf4 scikit-learn joblib>=0.12 chemfiles tqdm>=4.43.0 tidynamics>=1.0.0 rdkit>=2020.03.1 h5py==2.10.0" + MDA_PIP_MIN_DEPS: 'coveralls coverage<5 pytest-cov pytest-xdist' + MDA_PIP_EXTRA_DEPS: 'duecredit parmed' + +jobs: + main_tests: + if: "github.repository == 'MDAnalysis/mdanalysis'" + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, ] + python-version: [3.6, 3.7, 3.8] + run_type: [FULL, ] + install_hole: [true, ] + codecov: [true, ] + include: + - name: macOS + os: macOS-latest + python-version: 3.7 + run_type: FULL + install_hole: true + codecov: true + - name: minimal-ubuntu + os: ubuntu-latest + python-version: 3.6 + run_type: MIN + install_hole: false + codecov: true + - name: numpy_min + os: ubuntu-latest + python-version: 3.6 + run_type: FULL + install_hole: false + codecov: false + numpy: numpy=1.16.0 + - name: asv_check + os: ubuntu-latest + python-version: 3.7 + run_type: FULL + install_hole: false + codecov: false + env: + CYTHON_TRACE_NOGIL: 1 + MPLBACKEND: agg + GH_OS: ${{ matrix.os }} + + steps: + - uses: actions/checkout@v2 + + - name: setup_osx + if: startsWith(matrix.os, 'macOS') + run: | + # Set OS specific vars and compiler flags + echo "OS_NAME=osx" >> $GITHUB_ENV + # TODO: work out why this is necessary (from CI helpers) + echo "MACOSX_DEPLOYMENT_TARGET=10.9" >> $GITHUB_ENV + ulimit -S -n 2048 + clang -v + echo "CC=clang" >> $GITHUB_ENV + clang++ -v + echo "CXX=clang++" >> $GITHUB_ENV + gfortran-9 -v + echo "FC=gfortran-9" >> $GITHUB_ENV + + - name: setup_linux + if: startsWith(matrix.os, 'ubuntu') + run: | + # Set OS specific vars and compiler flags + echo "OS_NAME=linux" >> $GITHUB_ENV + gcc -v + echo "CC=gcc" >> $GITHUB_ENV + g++ -v + echo "CXX=g++" >> $GITHUB_ENV + gfortran -v + echo "FC=gfortran" >> $GITHUB_ENV + + - name: setup_miniconda + uses: conda-incubator/setup-miniconda@v2 + with: + python-version: ${{ matrix.python-version }} + auto-update-conda: true + channel-priority: flexible + channels: biobuilds, conda-forge + add-pip-as-python-dependency: true + # TODO: mamba causes pip to segfault, switch when fixed + #mamba-version: "*" + architecture: x64 + + - name: install_deps + env: + MDA_CONDA_FULL_DEPS: "${{ env.MDA_CONDA_MIN_DEPS }} ${{ env.MDA_CONDA_EXTRA_DEPS }}" + MDA_PIP_FULL_DEPS: "${{ env.MDA_PIP_MIN_DEPS }} ${{ env.MDA_PIP_EXTRA_DEPS }}" + run: | + # NOTE: vars need to be re-assigned + # NOTE: set matrix.numpy to pin to a specific numpy version + conda_deps="${{ matrix.numpy }} ${MDA_CONDA_${{ matrix.run_type }}_DEPS}" + pip_deps=${MDA_PIP_${{ matrix.run_type }}_DEPS} + conda install ${conda_deps} + pip install ${pip_deps} + + # also install asv if required + if [ ${{ matrix.name }} = "asv_check" ]; then + pip install asv + fi + + - name: check_setup + run: | + # Check OS and python setup + echo "OS: ${OS_NAME}" + which python + which pip + pip list + conda info + conda list + + - name: install_hole + if : matrix.install_hole + run: | + # We manually build hole2 to avoid OS incompatibilities + git clone https://github.com/MDAnalysis/hole2.git + cd hole2/src + source ../source.apache + (make FC=${FC}) && (make PREFIX=${HOME}/hole2 FC=${FC} install) + source ../source.unset + echo "HOLE_BINDIR=${HOME}/hole2/bin" >> $GITHUB_ENV + echo "${HOME}/hole2/bin" >> $GITHUB_PATH + + - name: install_mda + run: | + # TODO: using install instead of develop here causes coverage to drop + # for .pyx file. If possible a solution for this should be found. + (cd package/ && python setup.py develop) && (cd testsuite/ && python setup.py install) + + - name: run_tests + if: contains(matrix.name, 'asv_check') != true + run: | + PYTEST_FLAGS="--disable-pytest-warnings --durations=50" + if [ ${{ matrix.codecov }} = "true" ]; then + PYTEST_FLAGS="${PYTEST_FLAGS} --cov=MDAnalysis --cov-report=xml" + fi + echo $PYTEST_FLAGS + pytest -n 2 testsuite/MDAnalysisTests $PYTEST_FLAGS + + - name: run_asv + if: contains(matrix.name, 'asv_check') + run: | + cd benchmarks + time python -m asv check -E existing + + - name: codecov + if: matrix.codecov + uses: codecov/codecov-action@v1 + with: + file: coverage.xml + fail_ci_if_error: True + verbose: True + + + build_docs: + if: "github.repository == 'MDAnalysis/mdanalysis'" + runs-on: ubuntu-latest + env: + CYTHON_TRACE_NOGIL: 1 + MPLBACKEND: agg + + steps: + - uses: actions/checkout@v2 + + - name: setup_miniconda + uses: conda-incubator/setup-miniconda@v2 + with: + python-version: 3.7 + auto-update-conda: true + channel-priority: flexible + channels: biobuilds, conda-forge + add-pip-as-python-dependency: true + architecture: x64 + + - name: install_deps + run: | + conda_deps="${{ env.MDA_CONDA_MIN_DEPS }} ${{ env.MDA_CONDA_EXTRA_DEPS}}" + pip_deps="${{ env.MDA_PIP_MIN_DEPS}} ${{ env.MDA_PIP_EXTRA_DEPS }} sphinx==1.8.5 sphinx-sitemap sphinx_rtd_theme msmb_theme==1.2.0" + conda install ${conda_deps} + pip install ${pip_deps} + + - name: install_mda + run: | + cd package && python setup.py develop + + - name: build_docs + run: | + cd package && python setup.py build_sphinx -E + + - name: deploy_docs + if: github.event_name != 'pull_request' + run: | + # place the deploy call here + echo "Oh, maple syrup roast parsnips [Richard Gowers]" + + + pylint_check: + if: "github.repository == 'MDAnalysis/mdanalysis'" + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v2 + + - name: setup_miniconda + uses: conda-incubator/setup-miniconda@v2 + with: + python-version: 3.7 + auto-update-conda: true + channel-priority: flexible + channels: conda-forge + add-pip-as-python-dependency: true + mamba-version: "*" + architecture: x64 + + - name: install + run: | + which pip + which python + pip install pylint + + - name: pylint + env: + PYLINTRC: package/.pylintrc + run: | + pylint --py3k package/MDAnalysis && pylint --py3k testsuite/MDAnalysisTests + + + pypi_check: + if: "github.repository == 'MDAnalysis/mdanalysis'" + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v2 + + - name: setup_miniconda + uses: conda-incubator/setup-miniconda@v2 + with: + python-version: 3.7 + auto-update-conda: true + channel-priority: flexible + channels: conda-forge + add-pip-as-python-dependency: true + mamba-version: "*" + architecture: x64 + + - name: install_conda + run: | + conda install setuptools cython numpy twine + + - name: install_mdanalysis + run: | + cd package && python setup.py sdist + + - name: check_package_build + run: | + DISTRIBUTION=$(ls -t1 package/dist/MDAnalysis-*.tar.gz | head -n 1) + test -n "${DISTRIBUTION}" || { echo "no distribution package/dist/MDAnalysis-*.tar.gz found"; exit 1; } + echo "twine check $DISTRIBUTION" + twine check $DISTRIBUTION diff --git a/maintainer/install_hole.sh b/maintainer/install_hole.sh deleted file mode 100755 index 8e2046ae310..00000000000 --- a/maintainer/install_hole.sh +++ /dev/null @@ -1,115 +0,0 @@ -#!/bin/bash -# -# Written by Oliver Beckstein, 2016 -# -# This script is placed into the Public Domain using the CC0 1.0 Public Domain -# Dedication https://creativecommons.org/publicdomain/zero/1.0/ -# -# -# NOTE: IF YOU RUN THIS SCRIPT YOU MUST BE ABLE TO COMPLY WITH THE HOLE -# NOT-FOR-PROFIT LICENSE. -# -# -# Install HOLE from http://www.holeprogram.org/ -# -# arguments -# OSNAME : linux | darwin -# PREFIX : "path/to/installdir" -# -# PREFIX can be relative to CWD or an absolute path; it will be created -# and HOLE unpacked under PREFIX (the tar file contains "hole2/...") -# -# -# HOLE v2.2004 is used under the terms of the 'HOLE END USER LICENCE AGREEMENT -# NOT-FOR-PROFIT VERSION' as provided in the installation tarball as file -# 'doc/Licence-not-for-profit.asciidoc' and see copy at -# https://github.com/MDAnalysis/mdanalysis/files/372246/Licence-not-for-profit.txt -# (recent as of 2016-07-19). -# - - -set -o errexit -o nounset - -SCRIPT=$(basename $0) - -# We still use the 2.2004 versions (academic only) because -# for the v2.2005 (Apache license) there are no pre-compiled binaries -# available at http://www.holeprogram.org/ as of 2018-02-22 - -# could switch to http://www.holeprogram.org/downloads/2.2.004/hole2-NotForProfit-2.2.004-Linux-i686.tar.gz -DOWNLOAD_URL_LINUX='https://www.holeprogram.org/downloads/2.2.004/hole2-NotForProfit-2.2.004-Linux-x86_64.tar.gz' -TARFILE_LINUX='hole2-NotForProfit-2.2.004-Linux-x86_64.tar.gz' - -# could switch to http://www.holeprogram.org/downloads/2.2.004/hole2-NotForProfit-2.2.004-Darwin-i386.tar.gz -DOWNLOAD_URL_DARWIN='https://www.holeprogram.org/downloads/2.2.004/hole2-NotForProfit-2.2.004-Darwin-i386.tar.gz' -TARFILE_DARWIN=hole2-NotForProfit-2.2.004-Darwin-i386.tar.gz - -# path to dir with executables in the current HOLE distribution -HOLE_EXE_DIR=hole2/exe - -function die () { - local msg="$1" err=$2 - echo "[${SCRIPT}] ERROR: $msg [$err]" - exit $err -} - -function is_native_executable () { - local filename="$1" OSNAME="$2" - file "${filename}" | grep -qi ${OFORMAT} - return $? -} - -OSNAME="$1" -case "$OSNAME" in - Linux|linux) - OSNAME=Linux - OFORMAT=Linux - DOWNLOAD_URL="${DOWNLOAD_URL_LINUX}" - TARFILE=${TARFILE_LINUX} - ;; - OSX|osx|Darwin|darwin) - OSNAME=Darwin - OFORMAT="Mach-O" - DOWNLOAD_URL="${DOWNLOAD_URL_DARWIN}" - TARFILE=${TARFILE_DARWIN} - ;; - *) - die "OS '${OSNAME}' not supported." 10;; -esac - -PREFIX="$2" -test -n "$PREFIX" || die "missing second argument PREFIX (installation directory)" 10 - -#------------------------------------------------------------ -# start installation -#------------------------------------------------------------ - -mkdir -p "$PREFIX" && cd "$PREFIX" || die "Failed to create and cd to $PREFIX" 1 -if ! test -f ${TARFILE}; then - echo "Downloading ${TARFILE} from ${DOWNLOAD_URL}..." - # fixing curl on travis/anaconda, see http://stackoverflow.com/a/31060428/334357 - export CURL_CA_BUNDLE=/etc/ssl/certs/ca-certificates.crt - curl -L ${DOWNLOAD_URL} -o ${TARFILE} || \ - die "Failed to download ${TARFILE} from ${DOWNLOAD_URL}" 1 -fi - -# only install the executables in HOLE_EXE_DIR -tar xvf ${TARFILE} ${HOLE_EXE_DIR} || die "Failed 'tar xvf ${TARFILE} ${HOLE_EXE_DIR}'" 1 - -MDA_HOLE_BINDIR="${PWD}/${HOLE_EXE_DIR}" -HOLE_EXE="${MDA_HOLE_BINDIR}/hole" - -test -d "${MDA_HOLE_BINDIR}" || { \ - echo "Content of ${PWD}:"; - ls -la; - die "no HOLE exe dir ${MDA_HOLE_BINDIR} in $PWD" 2; } -test -f "${HOLE_EXE}" || die "hole executable ${HOLE_EXE} not installed" 2 -is_native_executable ${HOLE_EXE} ${OFORMAT} || \ - { file ${HOLE_EXE}; \ - die "${HOLE_EXE} will not run on ${OSNAME} (object format ${OFORMAT})" 3; } - -echo "HOLE executables were installed into ${MDA_HOLE_BINDIR}" -echo ">>> export PATH=\${PATH}:${MDA_HOLE_BINDIR}" - -# repeat this line in .travis.yml -export PATH=${PATH}:${MDA_HOLE_BINDIR} diff --git a/testsuite/MDAnalysisTests/analysis/test_hole2.py b/testsuite/MDAnalysisTests/analysis/test_hole2.py index 380eceba95e..4dc98365da7 100644 --- a/testsuite/MDAnalysisTests/analysis/test_hole2.py +++ b/testsuite/MDAnalysisTests/analysis/test_hole2.py @@ -480,7 +480,7 @@ def test_over_order_parameters(self, hole): assert key == rmsd idx = np.argsort(op) - arr = np.array(list(hole.profiles.values())) + arr = np.array(list(hole.profiles.values()), dtype=object) for op_prof, arr_prof in zip(profiles.values(), arr[idx]): assert op_prof is arr_prof @@ -496,7 +496,7 @@ def test_over_order_parameters_file(self, hole, tmpdir): assert key == rmsd idx = np.argsort(op) - arr = np.array(list(hole.profiles.values())) + arr = np.array(list(hole.profiles.values()), dtype=object) for op_prof, arr_prof in zip(profiles.values(), arr[idx]): assert op_prof is arr_prof @@ -520,7 +520,7 @@ def test_over_order_parameters_frames(self, hole): idx = np.argsort(op[:n_frames]) values = list(hole.profiles.values())[:n_frames] - arr = np.array(values) + arr = np.array(values, dtype=object) for op_prof, arr_prof in zip(profiles.values(), arr[idx]): assert op_prof is arr_prof From 27cfb8a06a7cb6f28b64bfe22c83db754436b577 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Fri, 7 May 2021 00:03:51 +0200 Subject: [PATCH 02/20] switch self._network to self.results.network --- .../hydrogenbonds/wbridge_analysis.py | 65 +++++++++++-------- .../MDAnalysisTests/analysis/test_wbridge.py | 62 +++++++++--------- 2 files changed, 68 insertions(+), 59 deletions(-) diff --git a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py index 2ad8d94565b..bb02ba0d40d 100644 --- a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py +++ b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py @@ -122,14 +122,14 @@ -------------------- For lower order water bridges, it might be desirable to represent the -connections as :attr:`WaterBridgeAnalysis.timeseries`. The results are returned -per frame and are a list of hydrogen bonds between the selection 1 or selection -2 and the bridging waters. Due to the complexity of the higher order water -bridge and the fact that one hydrogen bond between two waters can appear in -both third and fourth order water bridge, the hydrogen bonds in the -:attr:`WaterBridgeAnalysis.timeseries` attribute are generated in a depth-first -search manner to avoid duplication. Example code of how -:attr:`WaterBridgeAnalysis.timeseries` is generated:: +connections as :attr:`WaterBridgeAnalysis.results.timeseries`. The results +are returned per frame and are a list of hydrogen bonds between the selection +1 or selection 2 and the bridging waters. Due to the complexity of the higher +order water bridge and the fact that one hydrogen bond between two waters can +appear in both third and fourth order water bridge, the hydrogen bonds in the +:attr:`WaterBridgeAnalysis.results.timeseries` attribute are generated in a +depth-first search manner to avoid duplication. Example code of how +:attr:`WaterBridgeAnalysis.results.timeseries` is generated:: def network2timeseries(network, timeseries): '''Traverse the network in a depth-first fashion. @@ -307,7 +307,7 @@ class WaterBridgeAnalysis_OtherFF(WaterBridgeAnalysis): # 3 2 SOL HW2 # 4 3 ASP OD1 # 5 3 ASP OD2 - print(w.timeseries) + print(w.results.timeseries) prints out :: @@ -698,7 +698,7 @@ class WaterBridgeAnalysis(AnalysisBase): The analysis of the trajectory is performed with the :meth:`WaterBridgeAnalysis.run` method. The result is stored in - :attr:`WaterBridgeAnalysis.timeseries`. See + :attr:`WaterBridgeAnalysis.results.timeseries`. See :meth:`~WaterBridgeAnalysis.run` for the format. @@ -870,6 +870,10 @@ def __init__(self, universe, selection1='protein', consider setting the `update_selection` keywords to ``True`` to ensure correctness. + .. versionchanged: 2.0.0 + Deprecated :attr:`WaterBridgeAnalysis.timeseries` in favour of + :attr:`WaterBridgeAnalysis.results.timeseries` + .. versionchanged 0.20.0 The :attr:`WaterBridgeAnalysis.timeseries` has been updated see :attr:`WaterBridgeAnalysis.timeseries` for detail. @@ -921,7 +925,7 @@ def __init__(self, universe, selection1='protein', 'Invalid selection type {0!s}'.format( self.selection1_type)) - self._network = [] # final result accessed as self.network + self.results.network = [] # final result accessed as self.network self.timesteps = None # time for each frame self._log_parameters() @@ -1216,7 +1220,7 @@ def _single_frame(self): if self.update_water_selection: self._update_water_selection() else: - self._network.append(defaultdict(dict)) + self.results.network.append(defaultdict(dict)) return selection_1 = [] @@ -1365,12 +1369,12 @@ def traverse_water_network(graph, node, end, route, maxdepth, result): traverse_water_network(water_pool, next_mol, selection_2, route[:], self.order, result) - self._network.append(result['start']) + self.results.network.append(result['start']) def _traverse_water_network(self, graph, current, analysis_func=None, output=None, link_func=None, **kwargs): ''' - This function recursively traverses the water network self._network and + This function recursively traverses the water network self.results.network and finds the hydrogen bonds which connect the current atom to the next atom. The newly found hydrogen bond will be appended to the hydrogen bonds connecting the selection 1 to the current atom via link_func. @@ -1491,7 +1495,12 @@ def _generate_timeseries(self, output_format=None): w = WaterBridgeAnalysis(u) w.run() - timeseries = w.timeseries + timeseries = w.results.timeseries + + + .. versionchanged: 2.0.0 + Deprecated :attr:`WaterBridgeAnalysis.timeseries` in favour of + :attr:`WaterBridgeAnalysis.results.timeseries` .. versionchanged 0.20.0 The :attr:`WaterBridgeAnalysis.timeseries` has been updated where @@ -1506,7 +1515,7 @@ def analysis(current, output, *args, **kwargs): output = current timeseries = [] - for frame in self._network: + for frame in self.results.network: new_frame = [] self._traverse_water_network(frame, new_frame, analysis_func=analysis, @@ -1542,10 +1551,10 @@ def _get_network(self): .. versionadded:: 0.20.0 ''' - return self._network + return self.results.network def set_network(self, network): - self._network = network + self.results.network = network network = property(_get_network, set_network) @@ -1621,10 +1630,10 @@ def count_by_type(self, analysis_func=None, **kwargs): analysis_func = self._count_by_type_analysis output = 'combined' - if self._network: - length = len(self._network) + if self.results.network: + length = len(self.results.network) result_dict = defaultdict(int) - for frame in self._network: + for frame in self.results.network: frame_dict = defaultdict(int) self._traverse_water_network(frame, [], analysis_func=analysis_func, @@ -1671,9 +1680,9 @@ def count_by_time(self, analysis_func=None, **kwargs): """ if analysis_func is None: analysis_func = self._count_by_time_analysis - if self._network: + if self.results.network: result = [] - for time, frame in zip(self.timesteps, self._network): + for time, frame in zip(self.timesteps, self.results.network): result_dict = defaultdict(int) self._traverse_water_network(frame, [], analysis_func=analysis_func, @@ -1720,13 +1729,13 @@ def timesteps_by_type(self, analysis_func=None, **kwargs): analysis_func = self._timesteps_by_type_analysis output = 'combined' - if self._network: + if self.results.network: result = defaultdict(list) if self.timesteps is None: - timesteps = range(len(self._network)) + timesteps = range(len(self.results.network)) else: timesteps = self.timesteps - for time, frame in zip(timesteps, self._network): + for time, frame in zip(timesteps, self.results.network): self._traverse_water_network(frame, [], analysis_func=analysis_func, output=result, @@ -1754,10 +1763,10 @@ def generate_table(self, output_format=None): The output format of :attr:`~WaterBridgeAnalysis.table` can also be changed using output_format in a fashion similar to - :attr:`WaterBridgeAnalysis.timeseries` + :attr:`WaterBridgeAnalysis.results.timeseries` """ output_format = output_format or self.output_format - if self._network == []: + if self.results.network == []: msg = "No data computed, do run() first." warnings.warn(msg, category=MissingDataWarning) logger.warning(msg) diff --git a/testsuite/MDAnalysisTests/analysis/test_wbridge.py b/testsuite/MDAnalysisTests/analysis/test_wbridge.py index 8d0e9ee789e..675b1d07df4 100644 --- a/testsuite/MDAnalysisTests/analysis/test_wbridge.py +++ b/testsuite/MDAnalysisTests/analysis/test_wbridge.py @@ -274,15 +274,15 @@ def wb_multiframe(): wb = WaterBridgeAnalysis(u, 'protein and (resid 1)', 'protein and (resid 4)', order=4) # Build an dummy WaterBridgeAnalysis object for testing - wb._network = [] - wb._network.append({(1, 0, 12, None, 2.0, 180.0): None}) - wb._network.append({(0, None, 12, 13, 2.0, 180.0): None}) - wb._network.append({(1, 0, 3, None, 2.0, 180.0): + wb.results.network = [] + wb.results.network.append({(1, 0, 12, None, 2.0, 180.0): None}) + wb.results.network.append({(0, None, 12, 13, 2.0, 180.0): None}) + wb.results.network.append({(1, 0, 3, None, 2.0, 180.0): {(4, 2, 12, None, 2.0, 180.0): None}}) - wb._network.append({(0, None, 3, 2, 2.0, 180.0): + wb.results.network.append({(0, None, 3, 2, 2.0, 180.0): {(4, 2, 5, None, 2.0, 180.0): {(5, None, 11, 12, 2.0, 180.0): None}}}) - wb.timesteps = range(len(wb._network)) + wb.timesteps = range(len(wb.results.network)) return wb def test_nodata(self, universe_DA): @@ -310,21 +310,21 @@ def test_empty_selection(self, universe_DA): wb = WaterBridgeAnalysis(universe_DA, 'protein and (resid 9)', 'protein and (resid 10)', order=0) wb.run() - assert wb._network == [{}] + assert wb.results.network == [{}] def test_loop(self, universe_loop): '''Test if loop can be handled correctly''' wb = WaterBridgeAnalysis(universe_loop, 'protein and (resid 1)', 'protein and (resid 1 or resid 4)') wb.run() - assert_equal(len(wb._network[0].keys()), 2) + assert_equal(len(wb.results.network[0].keys()), 2) def test_donor_accepter(self, universe_DA): '''Test zeroth order donor to acceptor hydrogen bonding''' wb = WaterBridgeAnalysis(universe_DA, 'protein and (resid 1)', 'protein and (resid 4)', order=0, update_selection=True, debug=True) wb.run(verbose=False) - network = wb._network[0] + network = wb.results.network[0] assert_equal(list(network.keys())[0][:4], (1, 0, 2, None)) def test_donor_accepter_heavy(self, universe_DA): @@ -332,7 +332,7 @@ def test_donor_accepter_heavy(self, universe_DA): wb = WaterBridgeAnalysis(universe_DA, 'protein and (resid 1)', 'protein and (resid 4)', order=0, update_selection=True, debug=True, distance_type='heavy') wb.run(verbose=False) - network = wb._network[0] + network = wb.results.network[0] assert_equal(list(network.keys())[0][:4], (1, 0, 2, None)) def test_donor_accepter_pbc(self, universe_DA_PBC): @@ -340,7 +340,7 @@ def test_donor_accepter_pbc(self, universe_DA_PBC): wb = WaterBridgeAnalysis(universe_DA_PBC, 'protein and (resid 1)', 'protein and (resid 4)', order=0, pbc=True) wb.run(verbose=False) - network = wb._network[0] + network = wb.results.network[0] assert_equal(list(network.keys())[0][:4], (1, 0, 2, None)) def test_accepter_donor(self, universe_AD): @@ -348,7 +348,7 @@ def test_accepter_donor(self, universe_AD): wb = WaterBridgeAnalysis(universe_AD, 'protein and (resid 1)', 'protein and (resid 4)', order=0) wb.run(verbose=False) - network = wb._network[0] + network = wb.results.network[0] assert_equal(list(network.keys())[0][:4], (0, None, 1, 2)) def test_acceptor_water_accepter(self, universe_AWA): @@ -357,7 +357,7 @@ def test_acceptor_water_accepter(self, universe_AWA): wb = WaterBridgeAnalysis(universe_AWA, 'protein and (resid 1)', 'protein and (resid 4)') wb.run(verbose=False) - network = wb._network[0] + network = wb.results.network[0] assert_equal(list(network.keys())[0][:4], (0, None, 2, 1)) second = network[list(network.keys())[0]] assert_equal(list(second.keys())[0][:4], (3, 1, 4, None)) @@ -369,7 +369,7 @@ def test_donor_water_accepter(self, universe_DWA): wb = WaterBridgeAnalysis(universe_DWA, 'protein and (resid 1)', 'protein and (resid 4)') wb.run(verbose=False) - network = wb._network[0] + network = wb.results.network[0] assert_equal(list(network.keys())[0][:4], (1, 0, 2, None)) second = network[list(network.keys())[0]] assert_equal(list(second.keys())[0][:4], (3, 2, 4, None)) @@ -381,7 +381,7 @@ def test_acceptor_water_donor(self, universe_AWD): wb = WaterBridgeAnalysis(universe_AWD, 'protein and (resid 1)', 'protein and (resid 4)') wb.run(verbose=False) - network = wb._network[0] + network = wb.results.network[0] assert_equal(list(network.keys())[0][:4], (0, None, 2, 1)) second = network[list(network.keys())[0]] assert_equal(list(second.keys())[0][:4], (1, None, 3, 4)) @@ -393,7 +393,7 @@ def test_donor_water_donor(self, universe_DWD): wb = WaterBridgeAnalysis(universe_DWD, 'protein and (resid 1)', 'protein and (resid 4)') wb.run(verbose=False) - network = wb._network[0] + network = wb.results.network[0] assert_equal(list(network.keys())[0][:4], (1, 0, 2, None)) second = network[list(network.keys())[0]] assert_equal(list(second.keys())[0][:4], (2, None, 3, 4)) @@ -403,7 +403,7 @@ def test_empty(self, universe_empty): '''Test case where no water bridge exists''' wb = WaterBridgeAnalysis(universe_empty, 'protein', 'protein') wb.run(verbose=False) - assert_equal(wb._network[0], defaultdict(dict)) + assert_equal(wb.results.network[0], defaultdict(dict)) def test_same_selection(self, universe_DWA): ''' @@ -414,7 +414,7 @@ def test_same_selection(self, universe_DWA): wb = WaterBridgeAnalysis(universe_DWA, 'protein and resid 1', 'protein and resid 1') wb.run(verbose=False) - assert_equal(wb._network[0], defaultdict(dict)) + assert_equal(wb.results.network[0], defaultdict(dict)) def test_acceptor_2water_accepter(self, universe_AWWA): '''Test case where the hydrogen bond acceptor from selection 1 form second order @@ -423,12 +423,12 @@ def test_acceptor_2water_accepter(self, universe_AWWA): wb = WaterBridgeAnalysis(universe_AWWA, 'protein and (resid 1)', 'protein and (resid 4)') wb.run(verbose=False) - assert_equal(wb._network[0], defaultdict(dict)) + assert_equal(wb.results.network[0], defaultdict(dict)) # test second order wb = WaterBridgeAnalysis(universe_AWWA, 'protein and (resid 1)', 'protein and (resid 4)', order=2) wb.run(verbose=False) - network = wb._network[0] + network = wb.results.network[0] assert_equal(list(network.keys())[0][:4], (0, None, 2, 1)) second = network[list(network.keys())[0]] assert_equal(list(second.keys())[0][:4], (3, 1, 4, None)) @@ -439,7 +439,7 @@ def test_acceptor_2water_accepter(self, universe_AWWA): wb = WaterBridgeAnalysis(universe_AWWA, 'protein and (resid 1)', 'protein and (resid 4)', order=3) wb.run(verbose=False) - network = wb._network[0] + network = wb.results.network[0] assert_equal(list(network.keys())[0][:4], (0, None, 2, 1)) second = network[list(network.keys())[0]] assert_equal(list(second.keys())[0][:4], (3, 1, 4, None)) @@ -453,12 +453,12 @@ def test_acceptor_3water_accepter(self, universe_AWWWA): wb = WaterBridgeAnalysis(universe_AWWWA, 'protein and (resid 1)', 'protein and (resid 5)', order=2) wb.run(verbose=False) - assert_equal(wb._network[0], defaultdict(dict)) + assert_equal(wb.results.network[0], defaultdict(dict)) wb = WaterBridgeAnalysis(universe_AWWWA, 'protein and (resid 1)', 'protein and (resid 5)', order=3) wb.run(verbose=False) - network = wb._network[0] + network = wb.results.network[0] assert_equal(list(network.keys())[0][:4], (0, None, 2, 1)) second = network[list(network.keys())[0]] assert_equal(list(second.keys())[0][:4], (3, 1, 4, None)) @@ -471,7 +471,7 @@ def test_acceptor_3water_accepter(self, universe_AWWWA): wb = WaterBridgeAnalysis(universe_AWWWA, 'protein and (resid 1)', 'protein and (resid 5)', order=4) wb.run(verbose=False) - network = wb._network[0] + network = wb.results.network[0] assert_equal(list(network.keys())[0][:4], (0, None, 2, 1)) second = network[list(network.keys())[0]] assert_equal(list(second.keys())[0][:4], (3, 1, 4, None)) @@ -487,12 +487,12 @@ def test_acceptor_4water_accepter(self, universe_AWWWWA): wb = WaterBridgeAnalysis(universe_AWWWWA, 'protein and (resid 1)', 'protein and (resid 6)', order=3) wb.run(verbose=False) - assert_equal(wb._network[0], defaultdict(dict)) + assert_equal(wb.results.network[0], defaultdict(dict)) wb = WaterBridgeAnalysis(universe_AWWWWA, 'protein and (resid 1)', 'protein and (resid 6)', order=4) wb.run(verbose=False) - network = wb._network[0] + network = wb.results.network[0] assert_equal(list(network.keys())[0][:4], (0, None, 2, 1)) second = network[list(network.keys())[0]] assert_equal(list(second.keys())[0][:4], (3, 1, 4, None)) @@ -507,7 +507,7 @@ def test_acceptor_4water_accepter(self, universe_AWWWWA): wb = WaterBridgeAnalysis(universe_AWWWWA, 'protein and (resid 1)', 'protein and (resid 6)', order=5) wb.run(verbose=False) - network = wb._network[0] + network = wb.results.network[0] assert_equal(list(network.keys())[0][:4], (0, None, 2, 1)) second = network[list(network.keys())[0]] assert_equal(list(second.keys())[0][:4], (3, 1, 4, None)) @@ -526,7 +526,7 @@ def test_acceptor_22water_accepter(self, universe_branch): wb = WaterBridgeAnalysis(universe_branch, 'protein and (resid 1)', 'protein and (resid 4 or resid 5)', order=2) wb.run(verbose=False) - network = wb._network[0] + network = wb.results.network[0] assert_equal(list(network.keys())[0][:4], (0, None, 2, 1)) second = network[list(network.keys())[0]] assert_equal(list(second.keys())[0][:4], (3, 1, 4, None)) @@ -565,16 +565,16 @@ def test_acceptor_12water_accepter(self, universe_AWA_AWWA): wb = WaterBridgeAnalysis(universe_AWA_AWWA, 'protein and (resid 1 or resid 5)', 'protein and (resid 4 or resid 8)', order=1) wb.run(verbose=False) - network = wb._network[0] + network = wb.results.network[0] assert_equal(list(network.keys())[0][:4], (0, None, 2, 1)) second = network[list(network.keys())[0]] assert_equal(list(second.keys())[0][:4], (3, 1, 4, None)) assert_equal(second[list(second.keys())[0]], None) - network = wb._network[0] + network = wb.results.network[0] wb = WaterBridgeAnalysis(universe_AWA_AWWA, 'protein and (resid 1 or resid 5)', 'protein and (resid 4 or resid 8)', order=2) wb.run(verbose=False) - network = wb._network[0] + network = wb.results.network[0] assert_equal([(0, None, 2, 1), (5, None, 7, 6)], sorted([key[:4] for key in list(network.keys())])) From 17cd125bc4017d31e1077159794430a1fda5cabf Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Fri, 7 May 2021 00:29:54 +0200 Subject: [PATCH 03/20] add back deprecated attribute as property --- .../analysis/hydrogenbonds/wbridge_analysis.py | 18 +++++++++++++----- .../MDAnalysisTests/analysis/test_wbridge.py | 8 ++++++++ 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py index bb02ba0d40d..ce77a592be9 100644 --- a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py +++ b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py @@ -87,10 +87,10 @@ Since the waters connecting the two ends of the selections are by nature a network. We provide a network representation of the water network. Water bridge data are returned per frame, which is stored in -:attr:`WaterBridgeAnalysis.network`. Each frame is represented as a dictionary, -where the keys are the hydrogen bonds originating from selection 1 and the -values are new dictionaries representing the hydrogen bonds coming out of the -corresponding molecules making hydrogen bonds with selection 1. +:attr:`WaterBridgeAnalysis.results.network`. Each frame is represented as a +dictionary, where the keys are the hydrogen bonds originating from selection +1 and the values are new dictionaries representing the hydrogen bonds coming +out of the corresponding molecules making hydrogen bonds with selection 1. As for the hydrogen bonds which reach the selection 2, the values of the corresponding keys are None. One example where selection 1 and selection 2 are @@ -925,7 +925,7 @@ def __init__(self, universe, selection1='protein', 'Invalid selection type {0!s}'.format( self.selection1_type)) - self.results.network = [] # final result accessed as self.network + self.results.network = [] # final result accessed as self.results.network self.timesteps = None # time for each frame self._log_parameters() @@ -1813,3 +1813,11 @@ def generate_table(self, output_format=None): "WBridge: Stored results as table with %(num_records)d entries.", vars()) self.table = table + + @property + def _network(self): + wmsg = ("The `_network` attribute was deprecated in MDAnalysis 2.0.0 " + "and will be removed in MDAnalysis 3.0.0. Please use " + "`results.network` instead") + warnings.warn(wmsg, DeprecationWarning) + return self.results.network diff --git a/testsuite/MDAnalysisTests/analysis/test_wbridge.py b/testsuite/MDAnalysisTests/analysis/test_wbridge.py index 675b1d07df4..4188e7fcc92 100644 --- a/testsuite/MDAnalysisTests/analysis/test_wbridge.py +++ b/testsuite/MDAnalysisTests/analysis/test_wbridge.py @@ -723,3 +723,11 @@ def test_timesteps_by_type(self, wb_multiframe): timesteps = sorted(wb_multiframe.timesteps_by_type()) assert_array_equal(timesteps[3], [1, 12, 'ALA', 1, 'H', 'ALA', 6, 'O', 0, 2]) + + def test_warn_results_deprecated(self, universe_DA): + wb = WaterBridgeAnalysis(universe_DA, 'protein and (resid 9)', + 'protein and (resid 10)', order=0) + wb.run() + wmsg = "The `_network` attribute was deprecated in MDAnalysis 2.0.0" + with pytest.warns(DeprecationWarning, match=wmsg): + assert_equal(wb._network, wb.results.network) From c381c187c81ff904a31996e1c749d950749298a5 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Fri, 7 May 2021 00:34:52 +0200 Subject: [PATCH 04/20] update changelog --- package/CHANGELOG | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/package/CHANGELOG b/package/CHANGELOG index a94b9050120..44b110c48e9 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -174,6 +174,8 @@ Enhancements checking if it can be used in parallel analysis. (Issue #2996, PR #2950) Changes + * `hydrogenbonds.WaterBridgeAnalysis` now stores data using the + `WaterBridgeAnalysis.results.network` (Issue #3261, Issue #3270) * `contacts.Contacts` now stores data using the `Contacts.results.timeseries` attribute. `Contacts.timeseries` is now deprecated (Issue #3261) * `DensityAnalysis` now uses the `results.density` attribute for storing @@ -227,6 +229,12 @@ Changes * Added OpenMM coordinate and topology converters (Issue #2863, PR #2917) Deprecations + * The `hydrogenbonds.WaterBridgeAnalysis._network` attribute is now deprecated + in favour of `hydrogenbonds.WaterBridgeAnalysis.results.network`. It will be + removes in 3.0.0 (Issue #3261, Issue #3270) + * The `analysis.Contacts.timeseries` attribute is now deprecated in favour of + `analysis.Contacts.results.timeseries`. It will be removed in 3.0.0 + (Issue #3261) * The `analysis.Contacts.timeseries` attribute is now deprecated in favour of `analysis.Contacts.results.timeseries`. It will be removed in 3.0.0 (Issue #3261) From 04f53e3caac7702dd22ce607b46b8ebf100a2f79 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Fri, 7 May 2021 11:01:09 +0200 Subject: [PATCH 05/20] deprecate network property --- .../hydrogenbonds/wbridge_analysis.py | 39 ++++++------------- .../MDAnalysisTests/analysis/test_wbridge.py | 5 +++ 2 files changed, 17 insertions(+), 27 deletions(-) diff --git a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py index ce77a592be9..0a6c69456e4 100644 --- a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py +++ b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py @@ -1528,36 +1528,13 @@ def analysis(current, output, *args, **kwargs): timeseries = property(_generate_timeseries) - def _get_network(self): - r'''Network representation of the water network. - - The output is generated per frame as is explained in - :ref:`wb_Analysis_Network`. Each hydrogen bond has a compact - representation of :: - - [sele1_acceptor_idx, None, sele2_donor_idx, donor_heavy_idx, - distance, angle] - - or :: - - [sele1_donor_idx, donor_heavy_idx, sele1_acceptor_idx, None, - distance, angle] - - The donor_heavy_idx is the heavy atom bonding to the proton and atoms - can be retrived from the universe:: - - atom = u.atoms[idx] - - .. versionadded:: 0.20.0 - - ''' - return self.results.network - def set_network(self, network): + wmsg = ("The `set_network` method was deprecated in MDAnalysis 2.0.0 " + "and will be removed in MDAnalysis 3.0.0. Please use " + "`results.network` instead") + warnings.warn(wmsg, DeprecationWarning) self.results.network = network - network = property(_get_network, set_network) - @classmethod def _full_link(self, output, node): ''' @@ -1821,3 +1798,11 @@ def _network(self): "`results.network` instead") warnings.warn(wmsg, DeprecationWarning) return self.results.network + + @property + def network(self): + wmsg = ("The `network` attribute was deprecated in MDAnalysis 2.0.0 " + "and will be removed in MDAnalysis 3.0.0. Please use " + "`results.network` instead") + warnings.warn(wmsg, DeprecationWarning) + return self.results.network diff --git a/testsuite/MDAnalysisTests/analysis/test_wbridge.py b/testsuite/MDAnalysisTests/analysis/test_wbridge.py index 4188e7fcc92..023620ae0a5 100644 --- a/testsuite/MDAnalysisTests/analysis/test_wbridge.py +++ b/testsuite/MDAnalysisTests/analysis/test_wbridge.py @@ -728,6 +728,11 @@ def test_warn_results_deprecated(self, universe_DA): wb = WaterBridgeAnalysis(universe_DA, 'protein and (resid 9)', 'protein and (resid 10)', order=0) wb.run() + wmsg = "The `_network` attribute was deprecated in MDAnalysis 2.0.0" with pytest.warns(DeprecationWarning, match=wmsg): assert_equal(wb._network, wb.results.network) + + wmsg = "The `network` attribute was deprecated in MDAnalysis 2.0.0" + with pytest.warns(DeprecationWarning, match=wmsg): + assert_equal(wb.network, wb.results.network) From 7c3188012fee2dd744c7a39ae29acea525b9b740 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Fri, 7 May 2021 14:19:21 +0200 Subject: [PATCH 06/20] store table as result --- .../analysis/hydrogenbonds/wbridge_analysis.py | 10 +++++++++- testsuite/MDAnalysisTests/analysis/test_wbridge.py | 5 +++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py index 0a6c69456e4..4bfcd51ce99 100644 --- a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py +++ b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py @@ -1789,7 +1789,15 @@ def generate_table(self, output_format=None): logger.debug( "WBridge: Stored results as table with %(num_records)d entries.", vars()) - self.table = table + self.results.table = table + + @property + def table(self): + wmsg = ("The `table` attribute was deprecated in MDAnalysis 2.0.0 " + "and will be removed in MDAnalysis 3.0.0. Please use " + "`results.table` instead") + warnings.warn(wmsg, DeprecationWarning) + return self.results.table @property def _network(self): diff --git a/testsuite/MDAnalysisTests/analysis/test_wbridge.py b/testsuite/MDAnalysisTests/analysis/test_wbridge.py index 023620ae0a5..cc99623ac34 100644 --- a/testsuite/MDAnalysisTests/analysis/test_wbridge.py +++ b/testsuite/MDAnalysisTests/analysis/test_wbridge.py @@ -736,3 +736,8 @@ def test_warn_results_deprecated(self, universe_DA): wmsg = "The `network` attribute was deprecated in MDAnalysis 2.0.0" with pytest.warns(DeprecationWarning, match=wmsg): assert_equal(wb.network, wb.results.network) + + wb.generate_table() + wmsg = "The `table` attribute was deprecated in MDAnalysis 2.0.0" + with pytest.warns(DeprecationWarning, match=wmsg): + assert_equal(wb.table, wb.results.table) From 2229b58d9e446e5850374bba2d81d58e76a6f5f3 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Fri, 7 May 2021 14:35:50 +0200 Subject: [PATCH 07/20] cleanup --- package/CHANGELOG | 6 ++- .../hydrogenbonds/wbridge_analysis.py | 40 +++++++++++++------ 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index 44b110c48e9..ee12473ebc2 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -175,7 +175,8 @@ Enhancements Changes * `hydrogenbonds.WaterBridgeAnalysis` now stores data using the - `WaterBridgeAnalysis.results.network` (Issue #3261, Issue #3270) + `hydrogenbonds.WaterBridgeAnalysis.results` attribute + (Issue #3261, Issue #3270) * `contacts.Contacts` now stores data using the `Contacts.results.timeseries` attribute. `Contacts.timeseries` is now deprecated (Issue #3261) * `DensityAnalysis` now uses the `results.density` attribute for storing @@ -229,6 +230,9 @@ Changes * Added OpenMM coordinate and topology converters (Issue #2863, PR #2917) Deprecations + * The `hydrogenbonds.WaterBridgeAnalysis.table` attribute is now deprecated + in favour of `hydrogenbonds.WaterBridgeAnalysis.results.table`. It will be + removes in 3.0.0 (Issue #3261, Issue #3270) * The `hydrogenbonds.WaterBridgeAnalysis._network` attribute is now deprecated in favour of `hydrogenbonds.WaterBridgeAnalysis.results.network`. It will be removes in 3.0.0 (Issue #3261, Issue #3270) diff --git a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py index 4bfcd51ce99..14c986eb55d 100644 --- a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py +++ b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py @@ -122,14 +122,14 @@ -------------------- For lower order water bridges, it might be desirable to represent the -connections as :attr:`WaterBridgeAnalysis.results.timeseries`. The results +connections as :attr:`WaterBridgeAnalysis.timeseries`. The results are returned per frame and are a list of hydrogen bonds between the selection 1 or selection 2 and the bridging waters. Due to the complexity of the higher order water bridge and the fact that one hydrogen bond between two waters can appear in both third and fourth order water bridge, the hydrogen bonds in the -:attr:`WaterBridgeAnalysis.results.timeseries` attribute are generated in a +:attr:`WaterBridgeAnalysis.timeseries` attribute are generated in a depth-first search manner to avoid duplication. Example code of how -:attr:`WaterBridgeAnalysis.results.timeseries` is generated:: +:attr:`WaterBridgeAnalysis.timeseries` is generated:: def network2timeseries(network, timeseries): '''Traverse the network in a depth-first fashion. @@ -307,7 +307,7 @@ class WaterBridgeAnalysis_OtherFF(WaterBridgeAnalysis): # 3 2 SOL HW2 # 4 3 ASP OD1 # 5 3 ASP OD2 - print(w.results.timeseries) + print(w.timeseries) prints out :: @@ -678,6 +678,25 @@ def analysis(current, output, u, **kwargs): :attr:`~WaterBridgeAnalysis.timeseries` to find the specific time point of a water bridge existence. + .. attribute:: results.network + + .. versionadded:: 2.0.0 + + .. attribute:: network + + Alias to the :attr:`results.network` attribute. + + .. deprecated:: 2.0.0 + Will be removed in MDAnalysis 3.0.0. Please use + :attr:`results.hbonds` instead. + + .. attribute:: _network + + Alias to the :attr:`results.network` attribute. + + .. deprecated:: 2.0.0 + Will be removed in MDAnalysis 3.0.0. Please use + :attr:`results.hbonds` instead. """ from collections import defaultdict import logging @@ -698,7 +717,7 @@ class WaterBridgeAnalysis(AnalysisBase): The analysis of the trajectory is performed with the :meth:`WaterBridgeAnalysis.run` method. The result is stored in - :attr:`WaterBridgeAnalysis.results.timeseries`. See + :attr:`WaterBridgeAnalysis.timeseries`. See :meth:`~WaterBridgeAnalysis.run` for the format. @@ -872,7 +891,7 @@ def __init__(self, universe, selection1='protein', .. versionchanged: 2.0.0 Deprecated :attr:`WaterBridgeAnalysis.timeseries` in favour of - :attr:`WaterBridgeAnalysis.results.timeseries` + :attr:`WaterBridgeAnalysis.timeseries` .. versionchanged 0.20.0 The :attr:`WaterBridgeAnalysis.timeseries` has been updated @@ -1495,12 +1514,7 @@ def _generate_timeseries(self, output_format=None): w = WaterBridgeAnalysis(u) w.run() - timeseries = w.results.timeseries - - - .. versionchanged: 2.0.0 - Deprecated :attr:`WaterBridgeAnalysis.timeseries` in favour of - :attr:`WaterBridgeAnalysis.results.timeseries` + timeseries = w.timeseries .. versionchanged 0.20.0 The :attr:`WaterBridgeAnalysis.timeseries` has been updated where @@ -1740,7 +1754,7 @@ def generate_table(self, output_format=None): The output format of :attr:`~WaterBridgeAnalysis.table` can also be changed using output_format in a fashion similar to - :attr:`WaterBridgeAnalysis.results.timeseries` + :attr:`WaterBridgeAnalysis.timeseries` """ output_format = output_format or self.output_format if self.results.network == []: From c9444270382267ae1c80e924a27cec07685cd763 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Fri, 7 May 2021 14:47:15 +0200 Subject: [PATCH 08/20] test results.table --- .../analysis/hydrogenbonds/wbridge_analysis.py | 18 +++++++++++++++--- .../MDAnalysisTests/analysis/test_wbridge.py | 4 ++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py index 14c986eb55d..bfccce3ffef 100644 --- a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py +++ b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py @@ -697,6 +697,18 @@ def analysis(current, output, u, **kwargs): .. deprecated:: 2.0.0 Will be removed in MDAnalysis 3.0.0. Please use :attr:`results.hbonds` instead. + + .. attribute:: results.table + + .. versionadded:: 2.0.0 + + .. attribute:: table + + Alias to the :attr:`results.table` attribute. + + .. deprecated:: 2.0.0 + Will be removed in MDAnalysis 3.0.0. Please use + :attr:`results.table` instead. """ from collections import defaultdict import logging @@ -1750,10 +1762,10 @@ def generate_table(self, output_format=None): """Generate a normalised table of the results. The table is stored as a :class:`numpy.recarray` in the - attribute :attr:`~WaterBridgeAnalysis.table`. + attribute :attr:`~WaterBridgeAnalysis.results.table`. - The output format of :attr:`~WaterBridgeAnalysis.table` can also be - changed using output_format in a fashion similar to + The output format of :attr:`~WaterBridgeAnalysis.results.table` can also + be changed using output_format in a fashion similar to :attr:`WaterBridgeAnalysis.timeseries` """ output_format = output_format or self.output_format diff --git a/testsuite/MDAnalysisTests/analysis/test_wbridge.py b/testsuite/MDAnalysisTests/analysis/test_wbridge.py index cc99623ac34..68ca6e89995 100644 --- a/testsuite/MDAnalysisTests/analysis/test_wbridge.py +++ b/testsuite/MDAnalysisTests/analysis/test_wbridge.py @@ -711,12 +711,12 @@ def analysis(current, output, u): def test_generate_table_hba(self, wb_multiframe): '''Test generate table using hydrogen bond analysis format''' wb_multiframe.generate_table(output_format='donor_acceptor') - assert_array_equal(sorted(wb_multiframe.table.donor_resid), [1, 1, 2, 2, 2, 6, 6]) + assert_array_equal(sorted(wb_multiframe.results.table.donor_resid), [1, 1, 2, 2, 2, 6, 6]) def test_generate_table_s1s2(self, wb_multiframe): '''Test generate table using hydrogen bond analysis format''' wb_multiframe.generate_table(output_format='sele1_sele2') - assert_array_equal(sorted(wb_multiframe.table.sele1_resid), [1, 1, 1, 1, 2, 2, 3]) + assert_array_equal(sorted(wb_multiframe.results.table.sele1_resid), [1, 1, 1, 1, 2, 2, 3]) def test_timesteps_by_type(self, wb_multiframe): '''Test the timesteps_by_type function''' From 5e699ffc05844ac3108644813fed031980840f63 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Fri, 7 May 2021 14:51:19 +0200 Subject: [PATCH 09/20] more documentation --- .../MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py index bfccce3ffef..6daf06bb3e0 100644 --- a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py +++ b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py @@ -680,6 +680,8 @@ def analysis(current, output, u, **kwargs): .. attribute:: results.network + Network representation of the water network. + .. versionadded:: 2.0.0 .. attribute:: network @@ -700,6 +702,10 @@ def analysis(current, output, u, **kwargs): .. attribute:: results.table + Results re-formatted as a flat "normalised" table that is easier + to import into a database or dataframe for further processing. + Generated by the :meth:`WaterBridgeAnalysis.generate_table` method. + .. versionadded:: 2.0.0 .. attribute:: table From 0ca8171dc9467ba19de1b937d90f0812ffbff31a Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Fri, 7 May 2021 14:52:26 +0200 Subject: [PATCH 10/20] improve docs --- package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py index 6daf06bb3e0..21fbd21af1d 100644 --- a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py +++ b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py @@ -907,10 +907,6 @@ def __init__(self, universe, selection1='protein', consider setting the `update_selection` keywords to ``True`` to ensure correctness. - .. versionchanged: 2.0.0 - Deprecated :attr:`WaterBridgeAnalysis.timeseries` in favour of - :attr:`WaterBridgeAnalysis.timeseries` - .. versionchanged 0.20.0 The :attr:`WaterBridgeAnalysis.timeseries` has been updated see :attr:`WaterBridgeAnalysis.timeseries` for detail. From 3ebb1861b1caf05a1fc3902ce7b261e7bfe5eb74 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Fri, 7 May 2021 15:16:50 +0200 Subject: [PATCH 11/20] fix pep8 --- testsuite/MDAnalysisTests/analysis/test_wbridge.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/testsuite/MDAnalysisTests/analysis/test_wbridge.py b/testsuite/MDAnalysisTests/analysis/test_wbridge.py index b66e4a802b3..3ac9b011ac7 100644 --- a/testsuite/MDAnalysisTests/analysis/test_wbridge.py +++ b/testsuite/MDAnalysisTests/analysis/test_wbridge.py @@ -711,12 +711,18 @@ def analysis(current, output, u): def test_generate_table_hba(self, wb_multiframe): '''Test generate table using hydrogen bond analysis format''' wb_multiframe.generate_table(output_format='donor_acceptor') - assert_array_equal(sorted(wb_multiframe.results.table.donor_resid), [1, 1, 2, 2, 2, 6, 6]) + assert_array_equal( + sorted(wb_multiframe.results.table.donor_resid), + [1, 1, 2, 2, 2, 6, 6], + ) def test_generate_table_s1s2(self, wb_multiframe): '''Test generate table using hydrogen bond analysis format''' wb_multiframe.generate_table(output_format='sele1_sele2') - assert_array_equal(sorted(wb_multiframe.results.table.sele1_resid), [1, 1, 1, 1, 2, 2, 3]) + assert_array_equal( + sorted(wb_multiframe.results.table.sele1_resid), + [1, 1, 1, 1, 2, 2, 3], + ) def test_timesteps_by_type(self, wb_multiframe): '''Test the timesteps_by_type function''' From 3aacd7981de0e2a0f15e13ebdc303d78c83d3204 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Fri, 7 May 2021 18:15:18 +0200 Subject: [PATCH 12/20] fix pep8 --- .../hydrogenbonds/wbridge_analysis.py | 24 ++++++++++--------- .../MDAnalysisTests/analysis/test_wbridge.py | 2 +- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py index a58183f8a17..c333194a318 100644 --- a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py +++ b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py @@ -122,7 +122,7 @@ -------------------- For lower order water bridges, it might be desirable to represent the -connections as :attr:`WaterBridgeAnalysis.timeseries`. The results +connections as :attr:`WaterBridgeAnalysis.timeseries`. The results are returned per frame and are a list of hydrogen bonds between the selection 1 or selection 2 and the bridging waters. Due to the complexity of the higher order water bridge and the fact that one hydrogen bond between two waters can @@ -958,7 +958,8 @@ def __init__(self, universe, selection1='protein', 'Invalid selection type {0!s}'.format( self.selection1_type)) - self.results.network = [] # final result accessed as self.results.network + # final result accessed as self.results.network + self.results.network = [] self.timesteps = None # time for each frame self._log_parameters() @@ -1408,13 +1409,14 @@ def traverse_water_network(graph, node, end, route, maxdepth, result): def _traverse_water_network(self, graph, current, analysis_func=None, output=None, link_func=None, **kwargs): ''' - This function recursively traverses the water network self.results.network and - finds the hydrogen bonds which connect the current atom to the next - atom. The newly found hydrogen bond will be appended to the hydrogen - bonds connecting the selection 1 to the current atom via link_func. - When selection 2 is reached, the full list of hydrogen bonds - connecting the selection 1 to selection 2 will be fed into - analysis_func, which will then modify the output in place. + This function recursively traverses the water network + self.results.network and finds the hydrogen bonds which connect the + current atom to the next atom. The newly found hydrogen bond will be + appended to the hydrogen bonds connecting the selection 1 to the + current atom via link_func. When selection 2 is reached, the full list + of hydrogen bonds connecting the selection 1 to selection 2 will be + fed into analysis_func, which will then modify the output in place. + :param graph: The connection network describes the connection between the atoms in the water network. :param current: The hydrogen bonds from selection 1 until now. @@ -1767,8 +1769,8 @@ def generate_table(self, output_format=None): The table is stored as a :class:`numpy.recarray` in the attribute :attr:`~WaterBridgeAnalysis.results.table`. - The output format of :attr:`~WaterBridgeAnalysis.results.table` can also - be changed using output_format in a fashion similar to + The output format of :attr:`~WaterBridgeAnalysis.results.table` can + also be changed using output_format in a fashion similar to :attr:`WaterBridgeAnalysis.timeseries` """ output_format = output_format or self.output_format diff --git a/testsuite/MDAnalysisTests/analysis/test_wbridge.py b/testsuite/MDAnalysisTests/analysis/test_wbridge.py index 3ac9b011ac7..3825bc2e5e8 100644 --- a/testsuite/MDAnalysisTests/analysis/test_wbridge.py +++ b/testsuite/MDAnalysisTests/analysis/test_wbridge.py @@ -720,7 +720,7 @@ def test_generate_table_s1s2(self, wb_multiframe): '''Test generate table using hydrogen bond analysis format''' wb_multiframe.generate_table(output_format='sele1_sele2') assert_array_equal( - sorted(wb_multiframe.results.table.sele1_resid), + sorted(wb_multiframe.results.table.sele1_resid), [1, 1, 1, 1, 2, 2, 3], ) From 390c3a65d27fb7e6c3bb564800f8cb08f1bec18c Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Sun, 9 May 2021 22:15:29 +0200 Subject: [PATCH 13/20] fix comments related to table --- package/CHANGELOG | 13 ++--- .../hydrogenbonds/wbridge_analysis.py | 57 ++++++++++--------- .../MDAnalysisTests/analysis/test_wbridge.py | 13 ++--- 3 files changed, 40 insertions(+), 43 deletions(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index 2c04976a8dc..46f0ecd9737 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -239,13 +239,12 @@ Changes * Added OpenMM coordinate and topology converters (Issue #2863, PR #2917) Deprecations - * The `hydrogenbonds.WaterBridgeAnalysis.table` attribute is now deprecated - in favour of `hydrogenbonds.WaterBridgeAnalysis.results.table`. It will be - removes in 3.0.0 (Issue #3261, Issue #3270) - * The `hydrogenbonds.WaterBridgeAnalysis._network` attribute is now deprecated - in favour of `hydrogenbonds.WaterBridgeAnalysis.results.network`. It will be - removes in 3.0.0 (Issue #3261, Issue #3270) - * The `analysis.Contacts.timeseries` attribute is now deprecated in favour of + * The `hydrogenbonds.WaterBridgeAnalysis.table` attribute is now deprecated. + `generate_table()` will return `table`. + * The `hydrogenbonds.WaterBridgeAnalysis._network` attribute is now deprecated + in favour of `hydrogenbonds.WaterBridgeAnalysis.results.network`. It will be + removed in 3.0.0 (Issue #3261, Issue #3270) + * The `analysis.Contacts.timeseries` attribute is now deprecated in favour of `analysis.Contacts.results.timeseries`. It will be removed in 3.0.0 (Issue #3261) * The `analysis.rms.RMSD.rmsd` and `analysis.rms.RMSF.rmsf` attributes are diff --git a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py index c333194a318..27fce0ca259 100644 --- a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py +++ b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py @@ -700,21 +700,12 @@ def analysis(current, output, u, **kwargs): Will be removed in MDAnalysis 3.0.0. Please use :attr:`results.hbonds` instead. - .. attribute:: results.table - - Results re-formatted as a flat "normalised" table that is easier - to import into a database or dataframe for further processing. - Generated by the :meth:`WaterBridgeAnalysis.generate_table` method. - - .. versionadded:: 2.0.0 - .. attribute:: table - - Alias to the :attr:`results.table` attribute. - + .. deprecated:: 2.0.0 - Will be removed in MDAnalysis 3.0.0. Please use - :attr:`results.table` instead. + Will be removed in MDAnalysis 3.0.0. Please generate + the table with :meth:`generate_table` instead. + """ from collections import defaultdict import logging @@ -1524,7 +1515,7 @@ def _generate_timeseries(self, output_format=None): *index* one would use ``u.atoms[acceptor_index]``. The :attr:`timeseries` is a managed attribute and it is generated - from the underlying data in :attr:`_network` every time the + from the underlying data in :attr:`results.network` every time the attribute is accessed. It is therefore costly to call and if :attr:`timeseries` is needed repeatedly it is recommended that you assign to a variable:: @@ -1533,6 +1524,10 @@ def _generate_timeseries(self, output_format=None): w.run() timeseries = w.timeseries + .. versionchanged: 2.0.0 + + + .. versionchanged 0.20.0 The :attr:`WaterBridgeAnalysis.timeseries` has been updated where the donor and acceptor string has been changed to tuple @@ -1766,12 +1761,26 @@ def timesteps_by_type(self, analysis_func=None, **kwargs): def generate_table(self, output_format=None): """Generate a normalised table of the results. - The table is stored as a :class:`numpy.recarray` in the - attribute :attr:`~WaterBridgeAnalysis.results.table`. + Parameters + ---------- + output_format : {'sele1_sele2', 'donor_acceptor'} + The output format of the `table` can be changed a fashion similar to + :attr:`WaterBridgeAnalysis.results.timeseries` by changing the labels + of the columns of the participating atoms. + + Returns + ------- + table : numpy.recarray + A "tidy" table with one hydrogen bond per row, labeled according to + `output_format` and containing information of atom_1, atom_2, distance, + and angle. - The output format of :attr:`~WaterBridgeAnalysis.results.table` can - also be changed using output_format in a fashion similar to - :attr:`WaterBridgeAnalysis.timeseries` + .. versionchanged:: 2.0.0 + Return the generated table (as well as storing it as :attr:`table`). + + .. deprecated:: 2.0.0 + In release 3.0.0, :meth:`generate_table()` will _only_ return the table + and no longer store it in :attr:`table`. """ output_format = output_format or self.output_format if self.results.network == []: @@ -1820,15 +1829,9 @@ def generate_table(self, output_format=None): logger.debug( "WBridge: Stored results as table with %(num_records)d entries.", vars()) - self.results.table = table + self.table = table - @property - def table(self): - wmsg = ("The `table` attribute was deprecated in MDAnalysis 2.0.0 " - "and will be removed in MDAnalysis 3.0.0. Please use " - "`results.table` instead") - warnings.warn(wmsg, DeprecationWarning) - return self.results.table + return table @property def _network(self): diff --git a/testsuite/MDAnalysisTests/analysis/test_wbridge.py b/testsuite/MDAnalysisTests/analysis/test_wbridge.py index 3825bc2e5e8..256fb596d95 100644 --- a/testsuite/MDAnalysisTests/analysis/test_wbridge.py +++ b/testsuite/MDAnalysisTests/analysis/test_wbridge.py @@ -710,17 +710,17 @@ def analysis(current, output, u): def test_generate_table_hba(self, wb_multiframe): '''Test generate table using hydrogen bond analysis format''' - wb_multiframe.generate_table(output_format='donor_acceptor') + table = wb_multiframe.generate_table(output_format='donor_acceptor') assert_array_equal( - sorted(wb_multiframe.results.table.donor_resid), + sorted(table.donor_resid), [1, 1, 2, 2, 2, 6, 6], ) def test_generate_table_s1s2(self, wb_multiframe): '''Test generate table using hydrogen bond analysis format''' - wb_multiframe.generate_table(output_format='sele1_sele2') + table = wb_multiframe.generate_table(output_format='sele1_sele2') assert_array_equal( - sorted(wb_multiframe.results.table.sele1_resid), + sorted(table.sele1_resid), [1, 1, 1, 1, 2, 2, 3], ) @@ -765,8 +765,3 @@ def test_warn_results_deprecated(self, universe_DA): wmsg = "The `network` attribute was deprecated in MDAnalysis 2.0.0" with pytest.warns(DeprecationWarning, match=wmsg): assert_equal(wb.network, wb.results.network) - - wb.generate_table() - wmsg = "The `table` attribute was deprecated in MDAnalysis 2.0.0" - with pytest.warns(DeprecationWarning, match=wmsg): - assert_equal(wb.table, wb.results.table) From 5e8f3d5a69a9c230deb807bf673e29762dcef0f6 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Sun, 9 May 2021 22:21:32 +0200 Subject: [PATCH 14/20] fix comments related to _network --- package/CHANGELOG | 5 +++-- .../analysis/hydrogenbonds/wbridge_analysis.py | 18 +----------------- .../MDAnalysisTests/analysis/test_wbridge.py | 4 ---- 3 files changed, 4 insertions(+), 23 deletions(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index 46f0ecd9737..f5eb14a353a 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -175,6 +175,8 @@ Enhancements checking if it can be used in parallel analysis. (Issue #2996, PR #2950) Changes + * `hydrogenbonds.WaterBridgeAnalysis.generate_table()` now returns the table + instead of storing it as an attribute. * `hydrogenbonds.WaterBridgeAnalysis` now stores data using the `hydrogenbonds.WaterBridgeAnalysis.results` attribute (Issue #3261, Issue #3270) @@ -240,8 +242,7 @@ Changes Deprecations * The `hydrogenbonds.WaterBridgeAnalysis.table` attribute is now deprecated. - `generate_table()` will return `table`. - * The `hydrogenbonds.WaterBridgeAnalysis._network` attribute is now deprecated + * The `hydrogenbonds.WaterBridgeAnalysis.network` attribute is now deprecated in favour of `hydrogenbonds.WaterBridgeAnalysis.results.network`. It will be removed in 3.0.0 (Issue #3261, Issue #3270) * The `analysis.Contacts.timeseries` attribute is now deprecated in favour of diff --git a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py index 27fce0ca259..555eb609e8c 100644 --- a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py +++ b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py @@ -692,14 +692,6 @@ def analysis(current, output, u, **kwargs): Will be removed in MDAnalysis 3.0.0. Please use :attr:`results.hbonds` instead. - .. attribute:: _network - - Alias to the :attr:`results.network` attribute. - - .. deprecated:: 2.0.0 - Will be removed in MDAnalysis 3.0.0. Please use - :attr:`results.hbonds` instead. - .. attribute:: table .. deprecated:: 2.0.0 @@ -1713,7 +1705,7 @@ def timesteps_by_type(self, analysis_func=None, **kwargs): """Frames during which each water bridges existed, sorted by each water bridges. - Processes :attr:`WaterBridgeAnalysis._network` and returns a + Processes :attr:`WaterBridgeAnalysis.results.network` and returns a :class:`list` containing atom indices, residue names, residue numbers (from selection 1 and selection 2) and each timestep at which the water bridge was detected. @@ -1833,14 +1825,6 @@ def generate_table(self, output_format=None): return table - @property - def _network(self): - wmsg = ("The `_network` attribute was deprecated in MDAnalysis 2.0.0 " - "and will be removed in MDAnalysis 3.0.0. Please use " - "`results.network` instead") - warnings.warn(wmsg, DeprecationWarning) - return self.results.network - @property def network(self): wmsg = ("The `network` attribute was deprecated in MDAnalysis 2.0.0 " diff --git a/testsuite/MDAnalysisTests/analysis/test_wbridge.py b/testsuite/MDAnalysisTests/analysis/test_wbridge.py index 256fb596d95..b0bc5ab4890 100644 --- a/testsuite/MDAnalysisTests/analysis/test_wbridge.py +++ b/testsuite/MDAnalysisTests/analysis/test_wbridge.py @@ -758,10 +758,6 @@ def test_warn_results_deprecated(self, universe_DA): 'protein and (resid 10)', order=0) wb.run() - wmsg = "The `_network` attribute was deprecated in MDAnalysis 2.0.0" - with pytest.warns(DeprecationWarning, match=wmsg): - assert_equal(wb._network, wb.results.network) - wmsg = "The `network` attribute was deprecated in MDAnalysis 2.0.0" with pytest.warns(DeprecationWarning, match=wmsg): assert_equal(wb.network, wb.results.network) From 6ac9528fad4e4602d84cb5950a2d6547f964b2c5 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Sun, 9 May 2021 22:47:32 +0200 Subject: [PATCH 15/20] fix comments on timeseries --- .../hydrogenbonds/wbridge_analysis.py | 60 +++++++++++-------- .../MDAnalysisTests/analysis/test_wbridge.py | 10 +++- 2 files changed, 43 insertions(+), 27 deletions(-) diff --git a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py index 555eb609e8c..876b2de04e2 100644 --- a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py +++ b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py @@ -122,14 +122,14 @@ -------------------- For lower order water bridges, it might be desirable to represent the -connections as :attr:`WaterBridgeAnalysis.timeseries`. The results +connections as :attr:`WaterBridgeAnalysis.results.timeseries`. The results are returned per frame and are a list of hydrogen bonds between the selection 1 or selection 2 and the bridging waters. Due to the complexity of the higher order water bridge and the fact that one hydrogen bond between two waters can appear in both third and fourth order water bridge, the hydrogen bonds in the -:attr:`WaterBridgeAnalysis.timeseries` attribute are generated in a +:attr:`WaterBridgeAnalysis.results.timeseries` attribute are generated in a depth-first search manner to avoid duplication. Example code of how -:attr:`WaterBridgeAnalysis.timeseries` is generated:: +:attr:`WaterBridgeAnalysis.results.timeseries` is generated:: def network2timeseries(network, timeseries): '''Traverse the network in a depth-first fashion. @@ -307,7 +307,7 @@ class WaterBridgeAnalysis_OtherFF(WaterBridgeAnalysis): # 3 2 SOL HW2 # 4 3 ASP OD1 # 5 3 ASP OD2 - print(w.timeseries) + print(w.results.timeseries) prints out :: @@ -675,8 +675,8 @@ def analysis(current, output, u, **kwargs): .. attribute:: timesteps List of the times of each timestep. This can be used together with - :attr:`~WaterBridgeAnalysis.timeseries` to find the specific time point - of a water bridge existence. + :attr:`~WaterBridgeAnalysis.results.timeseries` to find the specific + time point of a water bridge existence. .. attribute:: results.network @@ -698,6 +698,20 @@ def analysis(current, output, u, **kwargs): Will be removed in MDAnalysis 3.0.0. Please generate the table with :meth:`generate_table` instead. + .. attribute:: results.timeseries + + List of hydrogen bonds between the selection 1 or selection 2 + and the bridging waters, for each frame. + + .. versionadded:: 2.0.0 + + .. attribute:: timeseries + + Alias to the :attr:`results.timeseries` attribute. + + .. deprecated:: 2.0.0 + Will be removed in MDAnalysis 3.0.0. Please use + :attr:`results.hbonds` instead. """ from collections import defaultdict import logging @@ -718,10 +732,9 @@ class WaterBridgeAnalysis(AnalysisBase): The analysis of the trajectory is performed with the :meth:`WaterBridgeAnalysis.run` method. The result is stored in - :attr:`WaterBridgeAnalysis.timeseries`. See + :attr:`WaterBridgeAnalysis.results.timeseries`. See :meth:`~WaterBridgeAnalysis.run` for the format. - .. versionadded:: 0.17.0 """ @@ -770,7 +783,7 @@ def __init__(self, universe, selection1='protein', universe. The timeseries is accessible as the attribute - :attr:`WaterBridgeAnalysis.timeseries`. + :attr:`WaterBridgeAnalysis.results.timeseries`. If no hydrogen bonds are detected or if the initial check fails, look at the log output (enable with :func:`MDAnalysis.start_logging` and set @@ -1506,20 +1519,6 @@ def _generate_timeseries(self, output_format=None): To find an acceptor atom in :attr:`Universe.atoms` by *index* one would use ``u.atoms[acceptor_index]``. - The :attr:`timeseries` is a managed attribute and it is generated - from the underlying data in :attr:`results.network` every time the - attribute is accessed. It is therefore costly to call and if - :attr:`timeseries` is needed repeatedly it is recommended that you - assign to a variable:: - - w = WaterBridgeAnalysis(u) - w.run() - timeseries = w.timeseries - - .. versionchanged: 2.0.0 - - - .. versionchanged 0.20.0 The :attr:`WaterBridgeAnalysis.timeseries` has been updated where the donor and acceptor string has been changed to tuple @@ -1544,7 +1543,6 @@ def analysis(current, output, *args, **kwargs): for entry in new_frame]) return timeseries - timeseries = property(_generate_timeseries) def set_network(self, network): wmsg = ("The `set_network` method was deprecated in MDAnalysis 2.0.0 " @@ -1780,6 +1778,7 @@ def generate_table(self, output_format=None): warnings.warn(msg, category=MissingDataWarning) logger.warning(msg) return None + timeseries = self._generate_timeseries(output_format) num_records = np.sum([len(hframe) for hframe in timeseries]) @@ -1825,6 +1824,11 @@ def generate_table(self, output_format=None): return table + + def _conclude(self): + self.results.timeseries = self._generate_timeseries() + + @property def network(self): wmsg = ("The `network` attribute was deprecated in MDAnalysis 2.0.0 " @@ -1832,3 +1836,11 @@ def network(self): "`results.network` instead") warnings.warn(wmsg, DeprecationWarning) return self.results.network + + @property + def timeseries(self): + wmsg = ("The `timeseries` attribute was deprecated in MDAnalysis 2.0.0 " + "and will be removed in MDAnalysis 3.0.0. Please use " + "`results.timeseries` instead") + warnings.warn(wmsg, DeprecationWarning) + return self.results.timeseries diff --git a/testsuite/MDAnalysisTests/analysis/test_wbridge.py b/testsuite/MDAnalysisTests/analysis/test_wbridge.py index b0bc5ab4890..913b11da776 100644 --- a/testsuite/MDAnalysisTests/analysis/test_wbridge.py +++ b/testsuite/MDAnalysisTests/analysis/test_wbridge.py @@ -540,7 +540,7 @@ def test_timeseries_wba(self, universe_branch): 'protein and (resid 4 or resid 5)', order=2) wb.output_format = 'sele1_sele2' wb.run(verbose=False) - timeseries = sorted(wb.timeseries[0]) + timeseries = sorted(wb.results.timeseries[0]) assert_equal(timeseries[0][:4], (0, 2, ('ALA', 1, 'O'), ('SOL', 2, 'HW1'))) assert_equal(timeseries[1][:4], (3, 4, ('SOL', 2, 'HW2'), ('SOL', 3, 'OW'))) @@ -553,7 +553,7 @@ def test_timeseries_hba(self, universe_branch): 'protein and (resid 4 or resid 5)', order=2) wb.output_format = 'donor_acceptor' wb.run(verbose=False) - timeseries = sorted(wb.timeseries[0]) + timeseries = sorted(wb.results.timeseries[0]) assert_equal(timeseries[0][:4], (2, 0, ('SOL', 2, 'HW1'), ('ALA', 1, 'O'))) assert_equal(timeseries[1][:4], (3, 4, ('SOL', 2, 'HW2'), ('SOL', 3, 'OW'))) @@ -751,7 +751,7 @@ def test_duplicate_water(self): wb = WaterBridgeAnalysis(u, 'resname LEU and name O', 'resname LEU and name N H', order=4) wb.run() - assert len(wb.timeseries[0]) == 2 + assert len(wb.results.timeseries[0]) == 2 def test_warn_results_deprecated(self, universe_DA): wb = WaterBridgeAnalysis(universe_DA, 'protein and (resid 9)', @@ -761,3 +761,7 @@ def test_warn_results_deprecated(self, universe_DA): wmsg = "The `network` attribute was deprecated in MDAnalysis 2.0.0" with pytest.warns(DeprecationWarning, match=wmsg): assert_equal(wb.network, wb.results.network) + + wmsg = "The `timeseries` attribute was deprecated in MDAnalysis 2.0.0" + with pytest.warns(DeprecationWarning, match=wmsg): + assert_equal(wb.timeseries, wb.results.timeseries) From 5ad1ab1f60d8b4954e98e25135c9c765dfe72b20 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Sun, 9 May 2021 22:48:24 +0200 Subject: [PATCH 16/20] Update package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py Co-authored-by: Oliver Beckstein --- package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py index 876b2de04e2..fc5332e0383 100644 --- a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py +++ b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py @@ -690,7 +690,7 @@ def analysis(current, output, u, **kwargs): .. deprecated:: 2.0.0 Will be removed in MDAnalysis 3.0.0. Please use - :attr:`results.hbonds` instead. + :attr:`results.network` instead. .. attribute:: table From 8ef725e5a48086fb1c7f5a06987029747923f8e6 Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Sun, 9 May 2021 23:00:39 +0200 Subject: [PATCH 17/20] fix pep8 --- .../hydrogenbonds/wbridge_analysis.py | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py index fc5332e0383..92fe4b5f99d 100644 --- a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py +++ b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py @@ -693,7 +693,7 @@ def analysis(current, output, u, **kwargs): :attr:`results.network` instead. .. attribute:: table - + .. deprecated:: 2.0.0 Will be removed in MDAnalysis 3.0.0. Please generate the table with :meth:`generate_table` instead. @@ -956,6 +956,7 @@ def __init__(self, universe, selection1='protein', # final result accessed as self.results.network self.results.network = [] + self.results.timeseries = None self.timesteps = None # time for each frame self._log_parameters() @@ -1754,23 +1755,23 @@ def generate_table(self, output_format=None): Parameters ---------- output_format : {'sele1_sele2', 'donor_acceptor'} - The output format of the `table` can be changed a fashion similar to - :attr:`WaterBridgeAnalysis.results.timeseries` by changing the labels - of the columns of the participating atoms. + The output format of the `table` can be changed a fashion similar + to :attr:`WaterBridgeAnalysis.results.timeseries` by changing the + labels of the columns of the participating atoms. Returns ------- table : numpy.recarray - A "tidy" table with one hydrogen bond per row, labeled according to - `output_format` and containing information of atom_1, atom_2, distance, - and angle. + A "tidy" table with one hydrogen bond per row, labeled according to + `output_format` and containing information of atom_1, atom_2, + distance, and angle. .. versionchanged:: 2.0.0 Return the generated table (as well as storing it as :attr:`table`). .. deprecated:: 2.0.0 - In release 3.0.0, :meth:`generate_table()` will _only_ return the table - and no longer store it in :attr:`table`. + In release 3.0.0, :meth:`generate_table()` will _only_ return the + table and no longer store it in :attr:`table`. """ output_format = output_format or self.output_format if self.results.network == []: @@ -1779,7 +1780,12 @@ def generate_table(self, output_format=None): logger.warning(msg) return None - timeseries = self._generate_timeseries(output_format) + if self.results.timeseries is not None \ + and output_format == self.output_format: + timeseries = self.results.timeseries + else: + # Recompute timeseries with correct output format + timeseries = self._generate_timeseries(output_format) num_records = np.sum([len(hframe) for hframe in timeseries]) # build empty output table @@ -1824,11 +1830,9 @@ def generate_table(self, output_format=None): return table - def _conclude(self): self.results.timeseries = self._generate_timeseries() - @property def network(self): wmsg = ("The `network` attribute was deprecated in MDAnalysis 2.0.0 " @@ -1839,8 +1843,8 @@ def network(self): @property def timeseries(self): - wmsg = ("The `timeseries` attribute was deprecated in MDAnalysis 2.0.0 " - "and will be removed in MDAnalysis 3.0.0. Please use " + wmsg = ("The `timeseries` attribute was deprecated in MDAnalysis " + "2.0.0 and will be removed in MDAnalysis 3.0.0. Please use " "`results.timeseries` instead") warnings.warn(wmsg, DeprecationWarning) return self.results.timeseries From a7ac42d0f9a2b0af98d29a0415445ffdc1f07fbe Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Sun, 9 May 2021 23:05:17 +0200 Subject: [PATCH 18/20] more pep8 fixes --- package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py index 92fe4b5f99d..0486a64ea91 100644 --- a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py +++ b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py @@ -1781,7 +1781,7 @@ def generate_table(self, output_format=None): return None if self.results.timeseries is not None \ - and output_format == self.output_format: + and output_format == self.output_format: timeseries = self.results.timeseries else: # Recompute timeseries with correct output format @@ -1831,7 +1831,7 @@ def generate_table(self, output_format=None): return table def _conclude(self): - self.results.timeseries = self._generate_timeseries() + self.results.timeseries = self._generate_timeseries() @property def network(self): From fff215581fb11a68b25516fc78a35c3b6c5d617c Mon Sep 17 00:00:00 2001 From: Rocco Meli Date: Mon, 10 May 2021 16:22:39 +0200 Subject: [PATCH 19/20] fix typo in documentation --- package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py index 0486a64ea91..b2973daefe5 100644 --- a/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py +++ b/package/MDAnalysis/analysis/hydrogenbonds/wbridge_analysis.py @@ -711,7 +711,7 @@ def analysis(current, output, u, **kwargs): .. deprecated:: 2.0.0 Will be removed in MDAnalysis 3.0.0. Please use - :attr:`results.hbonds` instead. + :attr:`results.timeseries` instead. """ from collections import defaultdict import logging From b15ec83974c44b8ed23c19518f33b51f0b37443c Mon Sep 17 00:00:00 2001 From: Oliver Beckstein Date: Mon, 10 May 2021 12:52:47 -0700 Subject: [PATCH 20/20] Apply suggestions from code review: CHANGELOG --- package/CHANGELOG | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/package/CHANGELOG b/package/CHANGELOG index 37526abe55c..4583cd70731 100644 --- a/package/CHANGELOG +++ b/package/CHANGELOG @@ -175,8 +175,8 @@ Enhancements checking if it can be used in parallel analysis. (Issue #2996, PR #2950) Changes - * `hydrogenbonds.WaterBridgeAnalysis.generate_table()` now returns the table - instead of storing it as an attribute. + * `hydrogenbonds.WaterBridgeAnalysis.generate_table()` now returns the table; as previously, + it also generates the `table` attribute but the attribute will be removed in 3.0.0. * `hydrogenbonds.WaterBridgeAnalysis` now stores data using the `hydrogenbonds.WaterBridgeAnalysis.results` attribute (Issue #3261, Issue #3270) @@ -255,7 +255,8 @@ Changes * Added OpenMM coordinate and topology converters (Issue #2863, PR #2917) Deprecations - * The `hydrogenbonds.WaterBridgeAnalysis.table` attribute is now deprecated. + * The `hydrogenbonds.WaterBridgeAnalysis.table` attribute is now deprecated and will + be removed in 3.0.0. * The `hydrogenbonds.WaterBridgeAnalysis.network` attribute is now deprecated in favour of `hydrogenbonds.WaterBridgeAnalysis.results.network`. It will be removed in 3.0.0 (Issue #3261, Issue #3270)