Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Towards a better optional package system #4345

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from
Draft

Towards a better optional package system #4345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
17b86c3
Towards a better optional package system
IAlibay Nov 14, 2023
9dfb1e4
fix some pep8 issues
IAlibay Nov 14, 2023
970a897
fix mypy
IAlibay Nov 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 11 additions & 12 deletions package/MDAnalysis/analysis/align.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,25 +196,24 @@

import numpy as np

try:
import Bio.AlignIO
import Bio.Align
import Bio.Align.Applications
except ImportError:
HAS_BIOPYTHON = False
else:
HAS_BIOPYTHON = True

import MDAnalysis as mda
import MDAnalysis.lib.qcprot as qcp
from MDAnalysis.exceptions import SelectionError, SelectionWarning
import MDAnalysis.analysis.rms as rms
from MDAnalysis.coordinates.memory import MemoryReader
from MDAnalysis.lib.util import get_weights
from MDAnalysis.lib.util import get_weights, optional_import
from MDAnalysis.lib.util import deprecate # remove 3.0

from .base import AnalysisBase

biopython = optional_import('Bio', min_version='1.80')

if biopython is not None:
import Bio.AlignIO
import Bio.Align
import Bio.Align.Applications


logger = logging.getLogger('MDAnalysis.analysis.align')


Expand Down Expand Up @@ -1070,7 +1069,7 @@ def sequence_alignment(mobile, reference, match_score=2, mismatch_penalty=-1,
Biopython is now an optional dependency which this method requires.

"""
if not HAS_BIOPYTHON:
if biopython is None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The semantics of if not HAS_BIOPYTHON are clearer.

The following is extra code

def has_optional_dependency(mod):
    return mod is not None

but may be more readable:

from MDAnalysis.lib.util import optional_import, has_optional_dependency

if not has_optional_dependency(biopython):
   # do the thing when we don't have biopython

errmsg = ("The `sequence_alignment` method requires an installation "
"of `Biopython`. Please install `Biopython` to use this "
"method: https://biopython.org/wiki/Download")
Expand Down Expand Up @@ -1194,7 +1193,7 @@ def fasta2select(fastafilename, is_aligned=False,
Biopython is now an optional dependency which this method requires.

"""
if not HAS_BIOPYTHON:
if biopython is None:
errmsg = ("The `fasta2select` method requires an installation "
"of `Biopython`. Please install `Biopython` to use this "
"method: https://biopython.org/wiki/Download")
Expand Down
60 changes: 60 additions & 0 deletions package/MDAnalysis/lib/util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,14 +200,18 @@
import gzip
import re
import io
import importlib
import warnings
import functools
from functools import wraps
import types
from typing import Optional
import textwrap
import weakref

import mmtf
import numpy as np
from packaging.version import Version

from numpy.testing import assert_equal
import inspect
Expand Down Expand Up @@ -2552,3 +2556,59 @@
self._kwargs[key] = arg
return func(self, *args, **kwargs)
return wrapper


def optional_import(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about a separate lib.optional module? It's a well-characterized problem so I'd prefer than throwing even more into util. We can then also have better/more focused docs to go along with it.

module_name: str,
min_version: Optional[str] = None,
max_version: Optional[str] = None
) -> Optional[types.ModuleType]:
"""
Optionally import modules, optionally checking the version

Parameters
----------
module_name : str
Name of the package to be optionally imported.
min_version : Optional[str]
The minimum package version. If ``None`` will not check the lower
version bound.
max_version : Optional[str]
The maximum package version. If ``None`` will not check the upper
version bound.

Returns
-------
module : Optional[types.ModuleType]
The imported module. If the module could not be imported, will
return ``None``.

Raises
------
ImportError
If the imported module version (assumed to be accessed under
``module.__version__``) does not fit within the upper or lower
bound set by `min_version` and `max_version`.
"""
def _check_version(
lower_version: Optional[str], upper_version: Optional[str]
):
if lower_version is None or upper_version is None:
return True
else:
return Version(lower_version) <= Version(upper_version)

try:
module = importlib.import_module(module_name)
except ModuleNotFoundError:
return None

if ((not _check_version(min_version, module.__version__)) or
(not _check_version(module.__version__, max_version))):
wmsg = (f"{module_name} version is {module.__version__} "

Check warning on line 2608 in package/MDAnalysis/lib/util.py

View check run for this annotation

Codecov / codecov/patch

package/MDAnalysis/lib/util.py#L2608 

Added line #L2608 was not covered by tests
f"and allowed version ranges are >= {min_version} "
f"<= {max_version}")
warnings.warn(wmsg)
return None

Check warning on line 2612 in package/MDAnalysis/lib/util.py

View check run for this annotation

Codecov / codecov/patch

package/MDAnalysis/lib/util.py#L2611-L2612 

Added lines #L2611 - L2612 were not covered by tests

return module
18 changes: 9 additions & 9 deletions testsuite/MDAnalysisTests/analysis/test_align.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

import MDAnalysis as mda
import MDAnalysis.analysis.align as align
from MDAnalysis.analysis.align import HAS_BIOPYTHON
from MDAnalysis.analysis.align import biopython
import MDAnalysis.analysis.rms as rms
import os
import numpy as np
Expand Down Expand Up @@ -501,13 +501,13 @@ class TestAlignmentProcessing:
seq = FASTA
error_msg = "selection string has unexpected length"

@pytest.mark.skipif(HAS_BIOPYTHON, reason='biopython is installed')
@pytest.mark.skipif(biopython is not None, reason='biopython is installed')
def test_importerror_biopython(self):
errmsg = "The `fasta2select` method requires an installation"
with pytest.raises(ImportError, match=errmsg):
_ = align.fasta2select(self.seq, is_aligned=True)

@pytest.mark.skipif(not HAS_BIOPYTHON, reason='requires biopython')
@pytest.mark.skipif(biopython is None, reason='requires biopython')
def test_fasta2select_aligned(self):
"""test align.fasta2select() on aligned FASTA (Issue 112)"""
sel = align.fasta2select(self.seq, is_aligned=True)
Expand All @@ -516,7 +516,7 @@ def test_fasta2select_aligned(self):
assert len(sel['mobile']) == 30623, self.error_msg

@pytest.mark.skipif(
executable_not_found("clustalw2") or not HAS_BIOPYTHON,
executable_not_found("clustalw2") or biopython is None,
reason="Test skipped because clustalw2 executable not found")
def test_fasta2select_file(self, tmpdir):
"""test align.fasta2select() on a non-aligned FASTA with default
Expand All @@ -528,7 +528,7 @@ def test_fasta2select_file(self, tmpdir):
assert len(sel['mobile']) == 23090, self.error_msg

@pytest.mark.skipif(
executable_not_found("clustalw2") or not HAS_BIOPYTHON,
executable_not_found("clustalw2") or biopython is None,
reason="Test skipped because clustalw2 executable not found")
def test_fasta2select_ClustalW(self, tmpdir):
"""MDAnalysis.analysis.align: test fasta2select() with ClustalW
Expand All @@ -543,7 +543,7 @@ def test_fasta2select_ClustalW(self, tmpdir):
assert len(sel['reference']) == 23080, self.error_msg
assert len(sel['mobile']) == 23090, self.error_msg

@pytest.mark.skipif(not HAS_BIOPYTHON, reason='requires biopython')
@pytest.mark.skipif(biopython is None, reason='requires biopython')
def test_fasta2select_resids(self, tmpdir):
"""test align.fasta2select() when resids provided (Issue #3124)"""
resids = [x for x in range(705)]
Expand All @@ -565,14 +565,14 @@ def atomgroups():
mobile = universe.select_atoms("resid 122-159")
return reference, mobile

@pytest.mark.skipif(HAS_BIOPYTHON, reason='biopython installed')
@pytest.mark.skipif(biopython is not None, reason='biopython installed')
def test_biopython_import_error(self, atomgroups):
ref, mob = atomgroups
errmsg = "The `sequence_alignment` method requires an installation of"
with pytest.raises(ImportError, match=errmsg):
align.sequence_alignment(mob, ref)

@pytest.mark.skipif(not HAS_BIOPYTHON, reason='requires biopython')
@pytest.mark.skipif(biopython is None, reason='requires biopython')
@pytest.mark.filterwarnings("ignore:`sequence_alignment` is deprecated!")
def test_sequence_alignment(self, atomgroups):
reference, mobile = atomgroups
Expand All @@ -588,7 +588,7 @@ def test_sequence_alignment(self, atomgroups):
assert score == pytest.approx(54.6)
assert_array_equal([begin, end], [0, reference.n_residues])

@pytest.mark.skipif(not HAS_BIOPYTHON, reason='requires biopython')
@pytest.mark.skipif(biopython is None, reason='requires biopython')
def test_sequence_alignment_deprecation(self, atomgroups):
reference, mobile = atomgroups
wmsg = ("`sequence_alignment` is deprecated!\n"
Expand Down
Loading