Repository: cogent3/cogent3
Pull request #1145 (2022-08-30 11:29:55)

DEV: Added non_composable to enum, removed composable argument from d…
…efine_app, excluded _add method for non_composable apps

Nick-Foto: @GavinHuttley 
Please review and merge these changes.
Please remind making a tag before merging this branch.
codecov[bot]: # [Codecov](https://codecov.io/gh/cogent3/cogent3/pull/1145?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cogent3) Report
Base: **91.34**% // Head: **91.33**% // Decreases project coverage by **`-0.01%`** :warning:
> Coverage data is based on head [(`cf2e68f`)](https://codecov.io/gh/cogent3/cogent3/pull/1145?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cogent3) compared to base [(`f72bc27`)](https://codecov.io/gh/cogent3/cogent3/commit/f72bc27c2415528d06c06470563d14c3ab370056?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cogent3).
> Patch coverage: 89.47% of modified lines in pull request are covered.

<details><summary>Additional details and impacted files</summary>


```diff
@@                   Coverage Diff                   @@
##           composable_refactor    #1145      +/-   ##
=======================================================
- Coverage                91.34%   91.33%   -0.02%     
=======================================================
  Files                      166      166              
  Lines                    31712    31716       +4     
=======================================================
- Hits                     28968    28967       -1     
- Misses                    2744     2749       +5     
```


| [Impacted Files](https://codecov.io/gh/cogent3/cogent3/pull/1145?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cogent3) | Coverage Δ | |
|---|---|---|
| [src/cogent3/app/composable.py](https://codecov.io/gh/cogent3/cogent3/pull/1145/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cogent3#diff-c3JjL2NvZ2VudDMvYXBwL2NvbXBvc2FibGUucHk=) | `90.37% <88.23%> (-0.19%)` | :arrow_down: |
| [src/cogent3/app/sample.py](https://codecov.io/gh/cogent3/cogent3/pull/1145/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cogent3#diff-c3JjL2NvZ2VudDMvYXBwL3NhbXBsZS5weQ==) | `96.43% <100.00%> (ø)` | |
| [...rc/cogent3/maths/matrix\_exponential\_integration.py](https://codecov.io/gh/cogent3/cogent3/pull/1145/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cogent3#diff-c3JjL2NvZ2VudDMvbWF0aHMvbWF0cml4X2V4cG9uZW50aWFsX2ludGVncmF0aW9uLnB5) | `92.72% <0.00%> (-1.82%)` | :arrow_down: |
| [src/cogent3/maths/scipy\_optimize.py](https://codecov.io/gh/cogent3/cogent3/pull/1145/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cogent3#diff-c3JjL2NvZ2VudDMvbWF0aHMvc2NpcHlfb3B0aW1pemUucHk=) | `84.14% <0.00%> (-0.65%)` | :arrow_down: |
| [src/cogent3/core/alignment.py](https://codecov.io/gh/cogent3/cogent3/pull/1145/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cogent3#diff-c3JjL2NvZ2VudDMvY29yZS9hbGlnbm1lbnQucHk=) | `95.12% <0.00%> (-0.06%)` | :arrow_down: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cogent3). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cogent3)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/cogent3/cogent3/pull/1145?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cogent3).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cogent3).


Patch:
diff --git a/src/cogent3/app/__init__.py b/src/cogent3/app/__init__.py
index fbfaebe4f..745f95d52 100644
--- a/src/cogent3/app/__init__.py
+++ b/src/cogent3/app/__init__.py
@@ -1,10 +1,9 @@
 import importlib
-import inspect
 
 
 __author__ = "Gavin Huttley"
 __copyright__ = "Copyright 2007-2022, The Cogent Project"
-__credits__ = ["Gavin Huttley"]
+__credits__ = ["Gavin Huttley", "Nick Shahmaras"]
 __license__ = "BSD-3"
 __version__ = "2022.8.24a1"
 __maintainer__ = "Gavin Huttley"
@@ -53,7 +52,7 @@ def available_apps():
     """returns Table listing the available apps"""
     from cogent3.util.table import Table
 
-    from .composable import Composable, __app_registry, user_function
+    from .composable import __app_registry
 
     # registration of apps does not happen until their modules are imported
     for name in __all__:
diff --git a/src/cogent3/app/composable.py b/src/cogent3/app/composable.py
index 8b903ddf4..4cf6944c9 100644
--- a/src/cogent3/app/composable.py
+++ b/src/cogent3/app/composable.py
@@ -773,12 +773,14 @@ class AppType(Enum):
     LOADER = "loader"
     WRITER = "writer"
     GENERIC = "generic"
+    NON_COMPOSABLE = "non_composable"
 
 
 # Aliases to use Enum easily
 LOADER = AppType.LOADER
 WRITER = AppType.WRITER
 GENERIC = AppType.GENERIC
+NON_COMPOSABLE = AppType.NON_COMPOSABLE
 
 
 def _get_raw_hints(main_func, min_params):
@@ -848,7 +850,7 @@ def _disconnect(self):
 
 
 def _add(self, other):
-    if other.input is not None:
+    if hasattr(other, "input") and other.input is not None:
         raise ValueError(
             f"{other.__class__.__name__} already part of composed function, use disconnect() to free them up"
         )
@@ -856,7 +858,9 @@ def _add(self, other):
         raise ValueError("cannot add an app to itself")
 
     # Check order
-    if self.app_type is WRITER:
+    if self.app_type is NON_COMPOSABLE or other.app_type is NON_COMPOSABLE:
+        raise TypeError("Both composed apps must be of composable type")
+    elif self.app_type is WRITER:
         raise TypeError("Left hand side of add operator must not be of type writer")
     elif other.app_type is LOADER:
         raise TypeError("Right hand side of add operator must not be of type loader")
@@ -1007,7 +1011,7 @@ def _class_from_func(func):
     return result
 
 
-def define_app(klass=None, *, app_type: AppType = GENERIC, composable: bool = True):
+def define_app(klass=None, *, app_type: AppType = GENERIC):
     """decorator for building callable apps
 
     Parameters
@@ -1017,15 +1021,13 @@ def define_app(klass=None, *, app_type: AppType = GENERIC, composable: bool = Tr
         class with the function bound as a static method.
     app_type
         what type of app, typically you just want GENERIC.
-    composable
-        whether the resulting app can be composed with others or not
 
     Notes
     -----
 
     Instances of ``cogent3`` apps are callable. If an exception occurs,
     the app returns a ``NotCompleted`` instance with logging information.
-    Apps defined with ``composable=True`` can be "composed" (summed together)
+    Apps defined with app_type LOADER, GENERIC or WRITER can be "composed" (summed together)
     to produce a single callable that sequentially invokes the composed
     apps. For example, the independent usage of app instances
     ``app1`` and ``app2`` as
@@ -1125,27 +1127,30 @@ def define_app(klass=None, *, app_type: AppType = GENERIC, composable: bool = Tr
 
         if not inspect.isclass(klass):
             raise ValueError(f"{klass} is not a class")
-        # check if user defined these functions
+
+        # check if user defined these methods
         method_list = [
             "__call__",
             "__repr__",
             "__str__",
             "__new__",
+            "_validate_data_type",
         ]
-        if composable:
-            method_list.extend(("__add__", "input", "disconnect", "apply_to"))
-
-        for meth in method_list:
-            if composable and inspect.isfunction(getattr(klass, meth, None)):
-                raise TypeError(
-                    f"remove {meth!r} method in {klass.__name__!r}, this functionality provided by define_app"
-                )
-        if composable and getattr(klass, "input", None):
+        excludes = ["__add__", "disconnect", "apply_to"]
+        if app_type is not NON_COMPOSABLE and getattr(klass, "input", None):
             raise TypeError(
                 f"remove 'input' attribute in {klass.__name__!r}, this functionality provided by define_app"
             )
+        elif app_type is not NON_COMPOSABLE:
+            method_list.extend(excludes)
 
-        for meth, func in __mapping.items():
+        for meth in method_list:
+            # make sure method not defined by user before adding
+            if inspect.isfunction(getattr(klass, meth, None)):
+                raise TypeError(
+                    f"remove {meth!r} in {klass.__name__!r}, this functionality provided by define_app"
+                )
+            func = __mapping["__repr__"] if meth == "__str__" else __mapping[meth]
             func.__name__ = meth
             setattr(klass, meth, func)
 
@@ -1155,7 +1160,7 @@ def define_app(klass=None, *, app_type: AppType = GENERIC, composable: bool = Tr
         setattr(klass, "_return_types", return_hint)
         setattr(klass, "app_type", app_type)
 
-        if app_type != LOADER:
+        if app_type is not LOADER:
             setattr(klass, "input", None)
 
         if hasattr(klass, "__slots__"):
@@ -1163,7 +1168,7 @@ def define_app(klass=None, *, app_type: AppType = GENERIC, composable: bool = Tr
             raise NotImplementedError("slots are not currently supported")
 
         # register this app
-        __app_registry[get_object_provenance(klass)] = composable
+        __app_registry[get_object_provenance(klass)] = app_type is not NON_COMPOSABLE
 
         return klass
 
@@ -1246,8 +1251,8 @@ def _apply_to(
         logger.log_versions(["cogent3"])
 
     results = []
-    process = self.input or self if self.app_type != LOADER else self
-    if self.app_type != LOADER and self.input:
+    process = self.input or self if self.app_type is not LOADER else self
+    if self.app_type is not LOADER and self.input:
         # As we will be explicitly calling the input object, we disconnect
         # the two-way interaction between input and self. This means self
         # is not called twice, and self is not unecessarily pickled during
diff --git a/src/cogent3/app/sample.py b/src/cogent3/app/sample.py
index b70dae5df..b2bc82069 100644
--- a/src/cogent3/app/sample.py
+++ b/src/cogent3/app/sample.py
@@ -8,14 +8,14 @@ from cogent3.core.alignment import Alignment, ArrayAlignment
 from cogent3.core.genetic_code import get_code
 from cogent3.core.moltype import get_moltype
 
-from .composable import NotCompleted, define_app
+from .composable import NON_COMPOSABLE, NotCompleted, define_app
 from .translate import get_fourfold_degenerate_sets
 from .typing import AlignedSeqsType, SeqsCollectionType, SerialisableType
 
 
 __author__ = "Gavin Huttley"
 __copyright__ = "Copyright 2007-2022, The Cogent Project"
-__credits__ = ["Gavin Huttley"]
+__credits__ = ["Gavin Huttley", "Nick Shahmaras"]
 __license__ = "BSD-3"
 __version__ = "2022.8.24a1"
 __maintainer__ = "Gavin Huttley"
@@ -40,7 +40,7 @@ def union(groups):
     return union
 
 
-@define_app(composable=False)
+@define_app(app_type=NON_COMPOSABLE)
 class concat:
     """Creates a concatenated alignment from a series."""
 


Test patch:
diff --git a/tests/test_app/test_composable.py b/tests/test_app/test_composable.py
index 0ace3450e..46dceebdf 100644
--- a/tests/test_app/test_composable.py
+++ b/tests/test_app/test_composable.py
@@ -1,3 +1,4 @@
+import inspect
 import os
 import pathlib
 import pickle
@@ -11,11 +12,18 @@ import pytest
 
 from scitrack import CachingLogger
 
+from cogent3 import make_aligned_seqs
+from cogent3.app import align, evo
 from cogent3.app import io as io_app
 from cogent3.app import sample as sample_app
+from cogent3.app import translate, tree
 from cogent3.app.composable import (
+    GENERIC,
+    NON_COMPOSABLE,
     NotCompleted,
     __app_registry,
+    _add,
+    _get_raw_hints,
     appify,
     define_app,
     get_object_provenance,
@@ -30,6 +38,7 @@ from cogent3.app.typing import (
     AlignedSeqsType,
     PairwiseDistanceType,
 )
+from cogent3.core.alignment import Alignment, SequenceCollection
 
 
 __author__ = "Gavin Huttley"
@@ -172,7 +181,6 @@ def test_disconnect():
 
 def test_apply_to():
     """correctly applies iteratively"""
-    from cogent3.core.alignment import SequenceCollection
 
     dstore = io_app.get_data_store("data", suffix="fasta", limit=3)
     reader = io_app.load_unaligned(format="fasta", moltype="dna")
@@ -376,17 +384,15 @@ def test_str():
 def test_composite_pickleable():
     """composable functions should be pickleable"""
 
-    from cogent3.app import align, evo, io, sample, translate, tree
-
-    read = io.load_aligned(moltype="dna")
+    read = io_app.load_aligned(moltype="dna")
     dumps(read)
     trans = translate.select_translatable()
     dumps(trans)
     aln = align.progressive_align("nucleotide")
     dumps(aln)
-    just_nucs = sample.omit_degenerates(moltype="dna")
+    just_nucs = sample_app.omit_degenerates(moltype="dna")
     dumps(just_nucs)
-    limit = sample.fixed_length(1000, random=True)
+    limit = sample_app.fixed_length(1000, random=True)
     dumps(limit)
     mod = evo.model("HKY85")
     dumps(mod)
@@ -410,9 +416,8 @@ def test_not_completed_result():
 def test_triggers_bugcatcher():
     """a composable that does not trap failures returns NotCompletedResult
     requesting bug report"""
-    from cogent3.app import io
 
-    read = io.load_aligned(moltype="dna")
+    read = io_app.load_aligned(moltype="dna")
     read.main = lambda x: None
     got = read("somepath.fasta")
     assert isinstance(got, NotCompleted)
@@ -446,7 +451,6 @@ for _app_ in (foo, bar):
 
 def test_user_function():
     """composable functions should be user definable"""
-    from cogent3 import make_aligned_seqs
 
     u_function = foo()
 
@@ -458,9 +462,6 @@ def test_user_function():
 
 def test_user_function_multiple():
     """user defined composable functions should not interfere with each other"""
-    from cogent3 import make_aligned_seqs
-    from cogent3.core.alignment import Alignment
-
     u_function_1 = foo()
     u_function_2 = bar()
 
@@ -591,7 +592,6 @@ def test_concat_not_composable():
 
 
 def test_composed_func_pickleable():
-    from cogent3.app.sample import min_length, omit_degenerates
 
     ml = min_length(100)
     no_degen = omit_degenerates(moltype="dna")
@@ -601,7 +601,7 @@ def test_composed_func_pickleable():
     assert unpickled.input is not None
 
 
-def test_composable_new1():
+def test_composable_variable_positional_args():
     """correctly associate argument vals with their names when have variable
     positional args"""
 
@@ -621,7 +621,62 @@ def test_composable_new1():
     __app_registry.pop(get_object_provenance(pos_var_pos1), None)
 
 
-def test_composable_new2():
+def test_composable_minimum_parameters():
+    """correctly associate argument vals with their names when have variable
+    positional args and kwargs"""
+
+    def test_func1(arg1) -> int:
+        return 1
+
+    with pytest.raises(ValueError):
+        _, _ = _get_raw_hints(test_func1, 2)
+
+
+def test_composable_return_type_hint():
+    """correctly associate argument vals with their names when have variable
+    positional args and kwargs"""
+
+    def test_func1(arg1):
+        return 1
+
+    with pytest.raises(TypeError):
+        _, _ = _get_raw_hints(test_func1, 1)
+
+
+def test_composable_firstparam_type_hint():
+    """correctly associate argument vals with their names when have variable
+    positional args and kwargs"""
+
+    def test_func1(arg1) -> int:
+        return 1
+
+    with pytest.raises(TypeError):
+        _, _ = _get_raw_hints(test_func1, 1)
+
+
+def test_composable_firstparam_type_is_None():
+    """correctly associate argument vals with their names when have variable
+    positional args and kwargs"""
+
+    def test_func1(arg1: None) -> int:
+        return 1
+
+    with pytest.raises(TypeError):
+        _, _ = _get_raw_hints(test_func1, 1)
+
+
+def test_composable_return_type_is_None():
+    """correctly associate argument vals with their names when have variable
+    positional args and kwargs"""
+
+    def test_func1(arg1: int) -> None:
+        return
+
+    with pytest.raises(TypeError):
+        _, _ = _get_raw_hints(test_func1, 1)
+
+
+def test_composable_variable_positional_args_and_kwargs():
     """correctly associate argument vals with their names when have variable
     positional args and kwargs"""
 
@@ -716,7 +771,6 @@ def func2app(arg1: int, exponent: int) -> float:
 
 def test_decorate_app_function():
     """works on functions now"""
-    import inspect
 
     sqd = func2app(exponent=2)
     assert sqd(3) == 9
@@ -727,7 +781,6 @@ def test_decorate_app_function():
 
 def test_roundtrip_decorated_function():
     """decorated function can be pickled/unpickled"""
-    import pickle
 
     sqd = func2app(exponent=2)
     u = pickle.loads(pickle.dumps(sqd))
@@ -737,7 +790,7 @@ def test_roundtrip_decorated_function():
 
 
 def test_decorated_func_optional():
-    @define_app(composable=False)
+    @define_app(app_type=NON_COMPOSABLE)
     def power(val: int, pow: int = 1) -> int:
         return val ** pow
 
@@ -748,7 +801,7 @@ def test_decorated_func_optional():
 
 
 def test_decorated_func_just_args():
-    @define_app(composable=False)
+    @define_app(app_type=NON_COMPOSABLE)
     def power(val: int, pow: int) -> int:
         return val ** pow
 
@@ -758,5 +811,82 @@ def test_decorated_func_just_args():
     __app_registry.pop(get_object_provenance(power), None)
 
 
+@pytest.mark.parametrize(
+    "meth",
+    [
+        "__call__",
+        "__repr__",
+        "__str__",
+        "__new__",
+        "__add__",
+        "disconnect",
+        "input",
+        "apply_to",
+        "_validate_data_type",
+    ],
+)
+def test_forbidden_methods_composable_app(meth):
+    class app_forbidden_methods1:
+        def __init__(self, a):
+            self.a = a
+
+        def main(self, val: int) -> int:
+            return val
+
+    def function1():
+        pass
+
+    setattr(app_forbidden_methods1, meth, function1)
+    with pytest.raises(TypeError):
+        define_app(app_type=GENERIC)(app_forbidden_methods1)
+
+
+@pytest.mark.parametrize(
+    "meth", ["__call__", "__repr__", "__str__", "__new__", "_validate_data_type"]
+)
+def test_forbidden_methods_non_composable_app(meth):
+    class app_forbidden_methods2:
+        def __init__(self, a):
+            self.a = a
+
+        def main(self, val: int) -> int:
+            return val
+
+    def function1():
+        pass
+
+    setattr(app_forbidden_methods2, meth, function1)
+    with pytest.raises(TypeError):
+        define_app(app_type=NON_COMPOSABLE)(app_forbidden_methods2)
+
+
+def test_aadd_non_composable_apps():
+    @define_app(app_type=NON_COMPOSABLE)
+    class app_non_composable1:
+        def __init__(self):
+            pass
+
+        def main(self, val: int) -> int:
+            return val
+
+    @define_app(app_type=NON_COMPOSABLE)
+    class app_non_composable2:
+        def __init__(self):
+            pass
+
+        def main(self, val: int) -> int:
+            return val
+
+    setattr(app_non_composable1, "__add__", _add)
+    setattr(app_non_composable2, "__add__", _add)
+    app1 = app_non_composable1()
+    app2 = app_non_composable2()
+    with pytest.raises(TypeError):
+        app1 + app2
+
+    __app_registry.pop(get_object_provenance(app_non_composable1), None)
+    __app_registry.pop(get_object_provenance(app_non_composable2), None)
+
+
 if __name__ == "__main__":
     main()
diff --git a/tests/test_app/test_data_store.py b/tests/test_app/test_data_store.py
index 6ece4fa60..b69cba82a 100644
--- a/tests/test_app/test_data_store.py
+++ b/tests/test_app/test_data_store.py
@@ -4,11 +4,14 @@ import pathlib
 import shutil
 import sys
 
+from copy import deepcopy
 from pathlib import Path
+from pickle import dumps, loads
 from tempfile import TemporaryDirectory
 from unittest import TestCase, main, skipIf
 
-from cogent3 import load_aligned_seqs
+from cogent3 import load_aligned_seqs, make_unaligned_seqs
+from cogent3.app.composable import NotCompleted
 from cogent3.app.data_store import (
     IGNORE,
     OVERWRITE,
@@ -20,10 +23,13 @@ from cogent3.app.data_store import (
     SingleReadDataStore,
     WritableDirectoryDataStore,
     WritableTinyDbDataStore,
+    _db_lockid,
     get_data_source,
     load_record_from_json,
 )
+from cogent3.app.io import load_db
 from cogent3.parse.fasta import MinimalFastaParser
+from cogent3.util.union_dict import UnionDict
 
 
 __author__ = "Gavin Huttley"
@@ -138,7 +144,6 @@ class DataStoreBaseReadTests:
 
     def test_pickleable_roundtrip(self):
         """pickling of data stores should be reversible"""
-        from pickle import dumps, loads
 
         dstore = self.ReadClass(self.basedir, suffix="*")
         re_dstore = loads(dumps(dstore))
@@ -147,7 +152,6 @@ class DataStoreBaseReadTests:
 
     def test_pickleable_member_roundtrip(self):
         """pickling of data store members should be reversible"""
-        from pickle import dumps, loads
 
         dstore = self.ReadClass(self.basedir, suffix="*")
         re_member = loads(dumps(dstore[0]))
@@ -442,7 +446,6 @@ class DirectoryDataStoreReadTests(
         """directory data store ignores"""
         with TemporaryDirectory(dir=".") as dirname:
             # tests the case when the directory has the file with the same suffix to self.suffix
-            from cogent3.app.composable import NotCompleted
 
             with TemporaryDirectory(dir=".") as dirname:
                 path = Path(dirname) / "subdir"
@@ -569,7 +572,6 @@ class TinyDBDataStoreTests(TestCase):
 
     def test_pickleable_roundtrip(self):
         """pickling of data stores should be reversible"""
-        from pickle import dumps, loads
 
         with TemporaryDirectory(dir=".") as dirname:
             path = os.path.join(dirname, "data")
@@ -589,10 +591,6 @@ class TinyDBDataStoreTests(TestCase):
 
     def test_unchanged_database_record(self):
         """tests unchanged record via the Readable and Writable DataStore interface to TinyDB"""
-        from copy import deepcopy
-
-        from cogent3.app.io import load_db
-
         loader = load_db()
         data = self.data
         original_record = deepcopy(data)
@@ -625,8 +623,6 @@ class TinyDBDataStoreTests(TestCase):
 
     def test_tiny_write_incomplete(self):
         """write an incomplete result to tinydb"""
-        from cogent3.app.composable import NotCompleted
-
         keys = list(self.data)
         incomplete = [
             keys.pop(0),
@@ -660,8 +656,6 @@ class TinyDBDataStoreTests(TestCase):
 
     def test_summary_methods(self):
         """produce a table"""
-        from cogent3.app.composable import NotCompleted
-
         keys = list(self.data)
         incomplete = [
             keys.pop(0),
@@ -690,9 +684,6 @@ class TinyDBDataStoreTests(TestCase):
 
     def test_dblock(self):
         """locking/unlocking of db"""
-
-        from cogent3.app.data_store import _db_lockid
-
         keys = list(self.data)
         with TemporaryDirectory(dir=".") as dirname:
             path = os.path.join(dirname, self.basedir)
@@ -868,8 +859,6 @@ class TestFunctions(TestCase):
 
     def test_get_data_source_seqcoll(self):
         """handles case where input is sequence collection object"""
-        from cogent3 import make_unaligned_seqs
-
         for val_klass in (str, pathlib.Path):
             value = val_klass("some/path.txt")
             obj = make_unaligned_seqs(
@@ -893,8 +882,6 @@ class TestFunctions(TestCase):
 
     def test_get_data_source_dict(self):
         """handles case where input is dict (sub)class instance with top level source key"""
-        from cogent3.util.union_dict import UnionDict
-
         for klass in (dict, UnionDict):
             for val_klass in (str, pathlib.Path):
                 value = val_klass("some/path.txt")
diff --git a/tests/test_app/test_dist.py b/tests/test_app/test_dist.py
index dfa7ba2c8..e180fb238 100644
--- a/tests/test_app/test_dist.py
+++ b/tests/test_app/test_dist.py
@@ -15,7 +15,7 @@ from cogent3.evolve.fast_distance import HammingPair, TN93Pair
 
 __author__ = "Gavin Huttley"
 __copyright__ = "Copyright 2007-2022, The Cogent Project"
-__credits__ = ["Gavin Huttley", "Stephen Ma"]
+__credits__ = ["Gavin Huttley", "Stephen Ma", "Nick Shahmaras"]
 __license__ = "BSD-3"
 __version__ = "2022.8.24a1"
 __maintainer__ = "Gavin Huttley"
@@ -49,7 +49,7 @@ _seqs5 = {"Human": "ASSLQHENSSLLLT", "Bandicoot": "XSLMLETSSLLSN"}
 
 
 def _get_all_composable_apps():
-    applications = [
+    return [
         align.align_to_ref(),
         align.progressive_align(model="GY94"),
         sample.fixed_length(100),
@@ -61,7 +61,6 @@ def _get_all_composable_apps():
         sample.take_named_seqs(),
         sample.trim_stop_codons(gc=1),
     ]
-    return applications
 
 
 class FastSlowDistTests(TestCase):
@@ -123,7 +122,7 @@ class FastSlowDistTests(TestCase):
         composable_apps = _get_all_composable_apps()
         calc_dist = dist_app.fast_slow_dist(fast_calc="hamming", moltype="dna")
         for app in composable_apps:
-            if app.app_type == WRITER:
+            if app.app_type is WRITER:
                 # cannot have a WRITER before a GENERIC
                 continue
             # Compose two composable applications, there should not be exceptions.
diff --git a/tests/test_app/test_evo.py b/tests/test_app/test_evo.py
index b0c9e2a30..71b0dfac9 100644
--- a/tests/test_app/test_evo.py
+++ b/tests/test_app/test_evo.py
@@ -20,7 +20,7 @@ from cogent3.util.deserialise import deserialise_object
 
 __author__ = "Gavin Huttley"
 __copyright__ = "Copyright 2007-2022, The Cogent Project"
-__credits__ = ["Gavin Huttley"]
+__credits__ = ["Gavin Huttley", "Nick Shahmaras"]
 __license__ = "BSD-3"
 __version__ = "2022.8.24a1"
 __maintainer__ = "Gavin Huttley"
@@ -55,7 +55,7 @@ class TestModel(TestCase):
             for value in (True, False):
                 # check setting via sm_args is overridden
                 with self.assertRaises(ValueError):
-                    model = evo_app.model(
+                    _ = evo_app.model(
                         mn,
                         optimise_motif_probs=value,
                         sm_args=dict(optimise_motif_probs=not value),
@@ -387,7 +387,7 @@ class TestModel(TestCase):
     def test_model_tree_unique_trees(self):
         """handles case of using unique trees for each alignment"""
         with self.assertRaises(AssertionError):
-            model1 = evo_app.model("GN", tree="(a,b,c)", unique_trees=True)
+            _ = evo_app.model("GN", tree="(a,b,c)", unique_trees=True)
         _data1 = {
             "Human": "ATGCGGCTCGCGGAGGCCGCGCTCGCGGAG",
             "Mouse": "ATGCCCGGCGCCAAGGCAGCGCTGGCGGAG",
diff --git a/tests/test_app/test_init.py b/tests/test_app/test_init.py
index b60cf49e5..9e235e12b 100644
--- a/tests/test_app/test_init.py
+++ b/tests/test_app/test_init.py
@@ -6,7 +6,7 @@ from unittest import TestCase, main
 
 from cogent3 import available_apps
 from cogent3.app import align, dist, evo, io, sample, translate, tree
-from cogent3.app.composable import LOADER, WRITER
+from cogent3.app.composable import LOADER, WRITER, is_composable
 
 
 __author__ = "Gavin Huttley"
@@ -25,7 +25,7 @@ def _get_all_composables(tmp_dir_name):
     test_hyp = evo.hypothesis(test_model1, test_model2)
     test_num_reps = 100
 
-    applications = [
+    return [
         align.align_to_ref(),
         align.progressive_align(model="GY94"),
         dist.fast_slow_dist(moltype="dna", fast_calc="hamming"),
@@ -51,7 +51,6 @@ def _get_all_composables(tmp_dir_name):
         tree.scale_branches(),
         tree.uniformize_tree(),
     ]
-    return applications
 
 
 class TestAvailableApps(TestCase):
@@ -65,11 +64,9 @@ class TestAvailableApps(TestCase):
 
     def test_composable_pairwise_applications(self):
         """Properly compose two composable applications"""
-        from cogent3.app.composable import is_composable
 
         with TemporaryDirectory(dir=".") as dirname:
             applications = _get_all_composables(os.path.join(dirname, "delme"))
-
             for app in applications:
                 self.assertTrue(is_composable(app), msg=app)
 
@@ -86,11 +83,11 @@ class TestAvailableApps(TestCase):
                 and app2.app_type is not LOADER
             ]
 
-            for composable_application_tuple in composable_application_tuples:
-                composable_application_tuple[0].disconnect()
-                composable_application_tuple[1].disconnect()
+            for app_a, app_b in composable_application_tuples:
+                app_a.disconnect()
+                app_b.disconnect()
                 # Compose two composable applications, there should not be exceptions.
-                composable_application_tuple[0] + composable_application_tuple[1]
+                app_a + app_b
 
             for app in applications:
                 if hasattr(app, "data_store"):
@@ -98,11 +95,9 @@ class TestAvailableApps(TestCase):
 
     def test_incompatible_pairwise_applications(self):
         """Properly identify two incompatible applications"""
-        from cogent3.app.composable import is_composable
 
         with TemporaryDirectory(dir=".") as dirname:
             applications = _get_all_composables(os.path.join(dirname, "delme"))
-
             for app in applications:
                 self.assertTrue(is_composable(app))
 
diff --git a/tests/test_app/test_io.py b/tests/test_app/test_io.py
index 4f5755661..757756b39 100644
--- a/tests/test_app/test_io.py
+++ b/tests/test_app/test_io.py
@@ -28,7 +28,7 @@ from cogent3.util.table import Table
 
 __author__ = "Gavin Huttley"
 __copyright__ = "Copyright 2007-2022, The Cogent Project"
-__credits__ = ["Gavin Huttley"]
+__credits__ = ["Gavin Huttley", "Nick Shahmaras"]
 __license__ = "BSD-3"
 __version__ = "2022.8.24a1"
 __maintainer__ = "Gavin Huttley"
@@ -233,7 +233,7 @@ class TestIo(TestCase):
             outpath = join(dirname, "delme")
             writer = write_db(outpath, create=True, if_exists="ignore")
             data = dict(a=[1, 2], b="string")
-            m = writer(data, identifier=join("blah", "delme.json"))
+            _ = writer(data, identifier=join("blah", "delme.json"))
             writer.data_store.db.close()
             dstore = io_app.get_data_store(f"{outpath}.tinydb", suffix="json")
             reader = io_app.load_db()
@@ -251,7 +251,6 @@ class TestIo(TestCase):
             gr = _get_generic_result(join("blah", "delme.json"))
             got = writer(gr)
             writer.data_store.db.close()
-            dstore = io_app.get_data_store(f"{outpath}.tinydb", suffix="json")
             reader = io_app.load_db()
             outpath = join(dirname, "dummy.json")
             with open(outpath, mode="w") as outfile:
@@ -467,7 +466,6 @@ class TestIo(TestCase):
 
         with TemporaryDirectory(dir=".") as dirname:
             outdir = join(dirname, "delme")
-
             obj = generic_result(source=join("blah", "delme.json"))
             obj["dna"] = DNA
             writer = io_app.write_json(outdir, create=True)
@@ -520,7 +518,7 @@ class TestIo(TestCase):
             writer = write_db(outdir, create=True, if_exists="overwrite")
             process = reader + aligner + writer
 
-            r = process.apply_to(
+            _ = process.apply_to(
                 members, show_progress=False, parallel=True, cleanup=True
             )
             expect = [str(m) for m in process.data_store]
diff --git a/tests/test_app/test_tree.py b/tests/test_app/test_tree.py
index b05eb141f..b049d31d6 100644
--- a/tests/test_app/test_tree.py
+++ b/tests/test_app/test_tree.py
@@ -184,7 +184,7 @@ class TestTree(TestCase):
         }
 
         darr = DistanceMatrix(data)
-        # must explicitly call main() method to avoid error trapping by Composable
+        # must explicitly call main() method to avoid error trapping by decorator
         with self.assertRaises(KeyError):
             quick_tree.main(darr)
         # when distance_matrix is None after dropping invalid
diff --git a/tests/test_app/test_typing.py b/tests/test_app/test_typing.py
index f3be0b668..10fe7b4df 100644
--- a/tests/test_app/test_typing.py
+++ b/tests/test_app/test_typing.py
@@ -1,4 +1,4 @@
-from typing import List, Tuple
+from typing import List, Tuple, Union
 
 import pytest
 
@@ -39,20 +39,16 @@ def test_get_constraint_names():
     assert got == {obj.__name__ for obj in (Alignment, ArrayAlignment)}
     got = get_constraint_names(UnalignedSeqsType)
     assert got == {SequenceCollection.__name__}
-
     got = get_constraint_names(SeqsCollectionType)
     assert got == {
         obj.__name__ for obj in (Alignment, ArrayAlignment, SequenceCollection)
     }
-
     got = get_constraint_names(TabularType)
-
     assert got == {obj.__name__ for obj in (Table, DictArray, DistanceMatrix)}
 
 
 def test_get_constraint_names_builtins():
     """handles built-in types"""
-    from typing import Union
 
     got = get_constraint_names(Union[str, bytes])
     assert got == {"str", "bytes"}
@@ -74,7 +70,6 @@ def test_get_constraint_names_identifiertype():
 
 def test_get_constraint_names_mixed_serilisable_identifiertype():
     """SerialisableType does not define any compatible types"""
-    from typing import Union
 
     got = get_constraint_names(Union[SerialisableType, IdentifierType, AlignedSeqsType])
     assert got == {"SerialisableType", "IdentifierType", "Alignment", "ArrayAlignment"}