Merge pull request #86 from OpenFreeEnergy/85-review-docstrings

85 review docstrings

examples/konnektor_networks.ipynb

@@ -76,7 +76,7 @@
     "Next it selects in the default variant the in average best transformation score performing `Component` as the central component.\n",
     "Finally all Components are connected with a `Transformation` to the central `Component`\n",
     "\n",
-    "The Star Netwrok is most edge efficient, but not most graph score efficient, as it has to find a central `Component`, which usually is a compromise for all 'Component's.\n",
+    "The Star Network is most edge efficient, but not most graph score efficient, as it has to find a central `Component`, which usually is a compromise for all 'Component's.\n",
     "From a robustness point of view, the Star Network, will immediatly be disconnected if one `Transformation` fails. \n",
     "However the loss of `Component`s is very limited, as only one ligand is lost per `Transformation` failure."
    ]

src/konnektor/network_planners/NetworkPlanner.py

@@ -16,8 +16,8 @@
 
 class NetworkPlanner(abc.ABC):
     def __init__(self, mappers: Union[AtomMapper, list[AtomMapper]], scorer):
-        """This class is an implementation for the LigandNetworkPlanner interface.
-        It defines the std. class for a Konnektor LigandNetworkPlanner
+        """This class is an implementation for the NetworkPlanner interface.
+        It defines the std. class for a Konnektor NetworkPlanner
 
         Parameters
         ----------

src/konnektor/network_planners/concatenators/_abstract_network_concatenator.py

@@ -28,12 +28,12 @@ def __init__(
         n_processes: int = 1,
         _initial_edge_lister=None,
     ):
-        """Base Class for the the LigandNetworkConcatenator classes.
-         It defines the std. class for a Konnektor LigandNetworkPlanner
+        """Base Class for the NetworkConcatenator classes.
+         It defines the std. class for a Konnektor NetworkConcatenator.
 
         Parameters
         ----------
-        mapper : AtomMapper
+        mappers : AtomMapper
             the AtomMappers to use to propose mappings.  At least 1 required,
             but many can be given, in which case all will be tried to find the
             lowest score edges

src/konnektor/network_planners/concatenators/cyclic_concatenator.py

@@ -19,7 +19,7 @@
 class CyclicConcatenator(NetworkConcatenator):
     def __init__(
         self,
-        mapper: AtomMapper,
+        mappers: Union[AtomMapper, Iterable[AtomMapper]],
         scorer: AtomMappingScorer,
         n_connecting_cycles: int = 2,
         cycle_sizes: Union[int, list[int]] = 3,
@@ -32,12 +32,12 @@ def __init__(
 
         Parameters
         ----------
-        mapper: AtomMapper
-            the atom mapper is required, to define the connection
-            between two ligands.
+        mappers: AtomMapper
+            The AtomMapper(s) to use to propose mappings.  At least 1 required,
+            but many can be given, in which case all will be tried to find the
+            lowest score edges
         scorer: AtomMappingScorer
-            scoring function evaluating an atom mapping, and giving a
-            score between [0,1].
+            Any callable which takes a AtomMapping and returns a float between [0,1]
         n_connecting_cycles: int, optional
             build at least n cycles between th networks. (default: 2)
         cycle_sizes: Union[int, list[int]], optional
@@ -49,11 +49,11 @@ def __init__(
         """
         if _initial_edge_lister is None:
             _initial_edge_lister = MaxConcatenator(
-                mappers=mapper, scorer=scorer, n_processes=n_processes
+                mappers=mappers, scorer=scorer, n_processes=n_processes
             )
 
         super().__init__(
-            mappers=mapper,
+            mappers=mappers,
             scorer=scorer,
             network_generator=MstNetworkAlgorithm(),
             n_processes=n_processes,

src/konnektor/network_planners/concatenators/max_concatenator.py

@@ -24,19 +24,17 @@ def __init__(
         show_progress: bool = False,
     ):
         """
-        This concatenators is connnecting two Networks with all possible
-         mappings. This is usually most useful for initial edge listing.
+        A NetworkConcatenator that connects two Networks with all possible
+        mappings. This is usually most useful for initial edge listing.
 
         Parameters
         ----------
-        mapper: AtomMapper
-            the atom mapper is required, to define the connection
-             between two ligands.
+        mappers: AtomMapper
+            the atom mapper is required to define the connection
+            between two ligands.
         scorer: AtomMappingScorer
             scoring function evaluating an atom mapping, and giving a
             score between [0,1].
-        n_connecting_edges: int, optional
-            number of connecting edges. (default: 3)
         n_processes: int
             number of processes that can be used for the network generation.
             (default: 1)
@@ -60,7 +58,7 @@ def concatenate_networks(
         Parameters
         ----------
         ligand_networks: Iterable[LigandNetwork]
-            an iterable of ligand networks, that shall be connected.
+            An iterable of LigandNetworks to connect.
 
         Returns
         -------

src/konnektor/network_planners/concatenators/mst_concatenator.py

@@ -27,8 +27,8 @@ def __init__(
         _initial_edge_lister: NetworkConcatenator = None,
     ):
         """
-        This concatenators is connnecting two Networks with a kruskal like
-         approach up to the number of connecting edges.
+        A NetworkConcatenator that connects two Networks with a Kruskal-like
+        approach, up to the number of connecting edges.
 
         Parameters
         ----------
@@ -39,7 +39,7 @@ def __init__(
             scoring function evaluating an atom mapping, and giving a score
             between [0,1].
         n_connecting_edges: int, optional
-            number of connecting edges. (default: 2)
+            maximum number of connecting edges. (default: 2)
         n_processes: int
             number of processes that can be used for the network generation.
             (default: 1)
@@ -49,15 +49,15 @@ def __init__(
             scorer=scorer,
             network_generator=MstNetworkAlgorithm(),
             n_processes=n_processes,
-            _initial_edge_lister=None,
+            _initial_edge_lister=None,  ## TODO: should this be _initial_edge_lister?
         )
         self.n_connecting_edges = n_connecting_edges
 
     def concatenate_networks(
         self, ligand_networks: Iterable[LigandNetwork]
     ) -> LigandNetwork:
         """
-        concatenate the giving networks.
+        Concatenate the given networks.
 
         Parameters
         ----------

src/konnektor/network_planners/generators/_abstract_network_generator.py

@@ -3,9 +3,9 @@
 
 import abc
 import logging
-from typing import Iterable, Union
+from typing import Callable, Iterable, Union
 
-from gufe import AtomMapper
+from gufe import AtomMapper, AtomMapping
 from gufe import LigandNetwork, Component
 
 from konnektor.network_planners._networkx_implementations import (
@@ -23,33 +23,35 @@ class NetworkGenerator(NetworkPlanner):
     def __init__(
         self,
         mappers: Union[AtomMapper, list[AtomMapper]],
-        scorer,
-        network_generator: _AbstractNetworkAlgorithm,
+        scorer: Callable[[AtomMapping], float],
+        network_generator: _AbstractNetworkAlgorithm,  ## TODO: rename this to network_algorithm?
+
         n_processes: int = 1,
         progress: bool = False,
         _initial_edge_lister=None,
     ):
-        """This class is an implementation for the LigandNetworkPlanner interface.
-        It defines the std. class for a Konnektor LigandNetworkPlanner
+        """This class is an implementation for the NetworkGenerator interface.
+        It defines the std. class for a Konnektor NetworkGenerator.
 
         Parameters
         ----------
-        mapper : AtomMapper
+        mappers : AtomMapper
             the AtomMappers to use to propose mappings.  At least 1 required,
             but many can be given, in which case all will be tried to find the
             lowest score edges
         scorer : AtomMappingScorer
-            any callable which takes a AtomMapping and returns a float
+            Any callable which takes a AtomMapping and returns a float
+        network_generator: the network algorithm to use
         n_processes: int, optional
-            number of processes that can be used for the network generation. (default: 1)
+            Number of processes that can be used for the network generation. (default: 1)
         progress: bool, optional
-            if true a progress bar will be displayed. (default: False)
-        _initial_edge_lister: LigandNetworkPlanner, optional
-            this LigandNetworkPlanner is used to give the initial set of edges. For standard usage, the Maximal NetworPlanner is used.
+            If `True`, displays a progress bar. (default: False)
+        _initial_edge_lister: NetworkPlanner, optional
+            The NetworkPlanner to use to create the initial set of edges. For standard usage, the MaximalNetworkPlanner is used.
             However in large scale approaches, it might be interesting to use the heuristicMaximalNetworkPlanner. (default: None)
 
         """
-        # generic Network_Planner attribsd
+        # generic NetworkPlanner attribs
         super().__init__(mappers=mappers, scorer=scorer)
 
         # Konnektor specific variables
@@ -93,7 +95,7 @@ def generate_ligand_network(self, components: Iterable[Component]) -> LigandNetw
         Parameters
         ----------
         components : Iterable[Component]
-        the ligands to include in the Network
+            the ligands to include in the Network
 
         Returns
         -------

src/konnektor/network_planners/generators/_parallel_mapping_pattern.py

@@ -3,6 +3,7 @@
 
 import functools
 import multiprocessing as mult
+from typing import Callable
 
 from gufe import AtomMapper, AtomMapping
 from gufe import SmallMoleculeComponent
@@ -70,7 +71,7 @@ def thread_mapping(args) -> list[AtomMapping]:
 
 def _parallel_map_scoring(
     possible_edges: list[tuple[SmallMoleculeComponent, SmallMoleculeComponent]],
-    scorer: callable,
+    scorer: Callable[[AtomMapping], float],
     mappers: list[AtomMapper],
     n_processes: int,
     show_progress: bool = True,

src/konnektor/network_planners/generators/clustered_network_generator.py

@@ -40,7 +40,7 @@ def __init__(
         clusterer: ComponentsDiversityClusterer = ComponentsDiversityClusterer(
             featurize=RDKitFingerprintTransformer(), cluster=KMeans(n_clusters=3)
         ),
-        mappers: Union[AtomMapper, list[AtomMapper]] = None,
+        mappers: Union[AtomMapper, list[AtomMapper]] = None,  # include None in this union?
         scorer=None,
         n_processes: int = 1,
         progress: bool = False,
@@ -51,19 +51,19 @@ def __init__(
             The algorithm works as follows:
             1. Cluster `Component` s with the `clusterer` obj.
             2. Build sub-networks in the clusters using the `sub_network_planners`.
-            3. Concatenate all sub-networks using the `concatenator` in order to build the final network.
+            3. Concatenate all sub-networks using the `concatenator` to build the final network.
 
 
         Parameters
         ----------
-        clusterer: ComponentsDiversityClusterer
-            This class is seperating the `Component` s along the first dimension.
         sub_network_planners: Iterable[NetworkGenerator]
-            The clusters, are then seperatley translated to sub networks by the sub_network_planners
+            NetworkGenerator(s) used to translate clusters to sub-networks.
         concatenator: NetworkConcatenator
-            The `NetworkConcatenator` is connecting the different sub networks.
-        mapper:  Union[AtomMapper, list[AtomMapper]]
-            the atom mapper is required, to define the connection between two ligands, if only `NetworkConcatenator` or  `NetworkGenerator` classes are passed
+            A `NetworkConcatenator` used to connect sub-networks.
+        clusterer: ComponentsDiversityClusterer
+            Separates the `Component` s along the first dimension.
+        mappers:  Union[AtomMapper, list[AtomMapper]]
+            Defines the connection between two ligands if `NetworkConcatenator` s or  `NetworkGenerator` s are provided. Otherwise, (?) (default:None)
         scorer: AtomMappingScorer
             scoring function evaluating an `AtomMapping`, and giving a score between [0,1], if only `NetworkConcatenator` or `NetworkGenerator` classes are passed
         progress: bool, optional

src/konnektor/network_planners/generators/cyclic_network_generator.py

@@ -27,40 +27,45 @@ def __init__(
         _initial_edge_lister: NetworkGenerator = None,
     ):
         """
-        The `CyclicNetworkGenerator` is generating a network based on many network cycles. T
-        his is of interest for analyzing the uncertainty of FE Estimates along the thermodynamic cycles and possibly correct the estimates with cycle closure analysis.
+        A `NetworkGenerator` that generates a network based on many network cycles.
+        This is of interest for analyzing the uncertainty of FE Estimates along thermodynamic
+        cycles and possibly for correcting the estimates with cycle closure analysis.
 
         The greedy algorithm builds the network up from a nodewise perspective.
-        For each node, the algorithm generates all cycles of size `cycle_sizes` and assigns a score to each cylce as the sum of all sub-scores.
-        Next it selects the `node_present_in_cyces` best score perfoming and node diversity increasing (see below) cycles per node.
+        For each node, the algorithm generates all cycles of size `cycle_sizes` and assigns a score to each cycle as the sum of all sub-scores.
+        Next, it selects the `node_present_in_cycles` best score performing and node diversity increasing (see below) cycles per node.
         The set of selected `Transformations` constructs the graph.
-        The node diversity criterion is an addition, which biases to spread the cycles on the graph eaqually between all `Components`
+        The node diversity criterion is an addition which biases to spread the cycles on the graph equally between all `Components`.
 
-        The number of cylces, around each `Component` can be defined by `component_present_in_cycles` and allowed cylce size can be tweaked with `cycle_sizes`. For `cycle_sizes` either an integer for providing an expected cycle size (e.g. `3`) or a range of allowed cycle sizes (e.g. `[3,4]`).
+        The number of cycles around each `Component` can be defined by `component_present_in_cycles`
+        and the allowed cycle size can be tweaked with `cycle_sizes`.
 
-        This layout has a well distributed connectivity between all `Component` s which increases the robustness very well, but still allows for a better graph score then the Twin Star Network, as the connectivity distribution is biased not enforced.
-        The large number of cycles might be very useful for statical analysis.  Nevertheless, the network has an increased amount of `Transformation` s
+        This layout has well-distributed connectivity between all `Component` s, which increases the robustness very well,
+        but still allows for a better graph score then the Twin Star Network, as the connectivity distribution is biased and not enforced.
+        The large number of cycles might be very useful for statistical analysis.  Nevertheless, the network has an increased amount of `Transformation` s.
 
 
         Parameters
         ----------
         mappers : Union[AtomMapper, list[AtomMapper]]
-            the AtomMappers to use to propose mappings.  At least 1 required,
+            AtomMapper(s) to use to propose mappings.  At least 1 required,
             but many can be given, in which case all will be tried to find the
-            lowest score edges
+            lowest score edges.
         scorer : AtomMappingScorer
-            any callable which takes a AtomMapping and returns a float
+            Any callable which takes a AtomMapping and returns a float.
         node_present_in_cycles: int
-            the number of cycles, the node should be present in.
+            The number of cycles the node should be present in.
         cycle_sizes: Union[int, List[int]]
-            the cycle size in the graph, that is used for designing the graph.
+            The cycle size to be used for designing the graph.
             When providing a list[int], a range of sizes is allowed (e.g. `[3,4]`). (default: 3)
         n_processes: int, optional
-            number of processes that can be used for the network generation.
+            Number of processes that can be used for the network generation.
             (default: 1)
-        _initial_edge_lister: LigandNetworkPlanner, optional
-            this LigandNetworkPlanner is used to give the initial set of edges.
-            For standard usage, the Maximal NetworPlanner is used. (default: MaximalNetworkPlanner)
+        progress: bool, optional
+            If `True`, displays a progress bar. (default: False)
+        _initial_edge_lister: NetworkPlanner, optional
+            The NetworkPlanner used to give the initial set of edges.
+            For standard usage, the MaximalNetworkPlanner is used. (default: MaximalNetworkPlanner)
         """
 
         network_generator = nx_CNG(

src/konnektor/network_planners/generators/explicit_network_generator.py

@@ -25,11 +25,11 @@ def __init__(
         Parameters
         ----------
         mapper: AtomMapper
-            the atom mapper is required, to define the connection between two ligands.
+            Defines the connection between two ligands.
         scorer: AtomMappingScorer
             scoring function evaluating an atom mapping, and giving a score between [0,1].
         n_processes: int
-            number of processes used to build the ligand network
+            number of processes to use to build the ligand network
         progress: bool, optional
             if true a progress bar will be displayed. (default: False)
         """
@@ -48,7 +48,7 @@ def generate_ligand_network(
         """
         Create a network with pre-defined edges.
 
-        This class is can be used as initial_edge_lister
+        This can be used as initial_edge_lister
 
         Parameters
         ----------