From a95a8c90713a56cc5008e61ddd44b572d3d339c6 Mon Sep 17 00:00:00 2001 From: Roman Ludwig <48687784+rmnldwg@users.noreply.github.com> Date: Mon, 1 Apr 2024 13:04:11 +0200 Subject: [PATCH 1/9] refactor(uni): use pandas `map` instead of `apply` This saves us a couple of lines in the `load_patient_data` method and is more readable. --- lymph/models/unilateral.py | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/lymph/models/unilateral.py b/lymph/models/unilateral.py index 2d9662e..e5d9dd1 100644 --- a/lymph/models/unilateral.py +++ b/lymph/models/unilateral.py @@ -2,7 +2,7 @@ import warnings from itertools import product -from typing import Any, Iterable, Literal +from typing import Any, Callable, Iterable, Literal import numpy as np import pandas as pd @@ -489,7 +489,7 @@ def load_patient_data( self, patient_data: pd.DataFrame, side: str = "ipsi", - mapping: callable | dict[int, Any] | None = None, + mapping: Callable[[int], Any] | dict[int, Any] = early_late_mapping, ) -> None: """Load patient data in `LyProX`_ format into the model. @@ -509,10 +509,6 @@ def load_patient_data( .. _LyProX: https://lyprox.org/ """ - if mapping is None: - mapping = early_late_mapping - - # pylint: disable=unnecessary-lambda-assignment patient_data = ( patient_data .copy() @@ -545,15 +541,7 @@ def load_patient_data( patient_data["_model", modality, lnl] = column - if len(patient_data) == 0: - patient_data[MAP_T_COL] = None - else: - mapping = dict_to_func(mapping) if isinstance(mapping, dict) else mapping - patient_data[MAP_T_COL] = patient_data.apply( - lambda row: mapping(row[RAW_T_COL]), - axis=1, - ) - + patient_data[MAP_T_COL] = patient_data[RAW_T_COL].map(mapping) self._patient_data = patient_data self._cache_version += 1 From 4b03bb647b292286bb6c6950f3bc135928c47131 Mon Sep 17 00:00:00 2001 From: Roman Ludwig <48687784+rmnldwg@users.noreply.github.com> Date: Mon, 1 Apr 2024 13:07:26 +0200 Subject: [PATCH 2/9] fix(uni): `load_patient_data` should accept `None` --- lymph/models/unilateral.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lymph/models/unilateral.py b/lymph/models/unilateral.py index e5d9dd1..f2cc40c 100644 --- a/lymph/models/unilateral.py +++ b/lymph/models/unilateral.py @@ -489,7 +489,7 @@ def load_patient_data( self, patient_data: pd.DataFrame, side: str = "ipsi", - mapping: Callable[[int], Any] | dict[int, Any] = early_late_mapping, + mapping: Callable[[int], Any] | dict[int, Any] | None = None, ) -> None: """Load patient data in `LyProX`_ format into the model. @@ -509,6 +509,9 @@ def load_patient_data( .. _LyProX: https://lyprox.org/ """ + if mapping is None: + mapping = early_late_mapping + patient_data = ( patient_data .copy() From e58a52f7c59f91378320fcc7c8d00cd72c00c289 Mon Sep 17 00:00:00 2001 From: Roman Ludwig <48687784+rmnldwg@users.noreply.github.com> Date: Mon, 1 Apr 2024 13:45:38 +0200 Subject: [PATCH 3/9] fix(mid): correct type hint of `marginalize` --- lymph/models/midline.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lymph/models/midline.py b/lymph/models/midline.py index 0a31a0c..f95c40a 100644 --- a/lymph/models/midline.py +++ b/lymph/models/midline.py @@ -753,7 +753,7 @@ def posterior_state_dist( def marginalize( self, - involvement: types.PatternType | None = None, + involvement: dict[str, types.PatternType] | None = None, given_state_dist: np.ndarray | None = None, t_stage: str = "early", mode: Literal["HMM", "BN"] = "HMM", From 2fe265c51ad129c6f952917552911d7ea0f163c2 Mon Sep 17 00:00:00 2001 From: Roman Ludwig <48687784+rmnldwg@users.noreply.github.com> Date: Mon, 8 Apr 2024 17:34:22 +0200 Subject: [PATCH 4/9] remove: remains of callbacks Some callback functionality that was tested in a pre-release has been forgotten in the code base and is now deleted. --- lymph/graph.py | 16 +++------------- tests/edge_test.py | 13 +------------ tests/graph_representation_test.py | 5 ----- 3 files changed, 4 insertions(+), 30 deletions(-) diff --git a/lymph/graph.py b/lymph/graph.py index ffb79ac..3e92723 100644 --- a/lymph/graph.py +++ b/lymph/graph.py @@ -23,7 +23,6 @@ flatten, popfirst, set_params_for, - trigger, ) @@ -224,7 +223,6 @@ def __init__( child: LymphNodeLevel, spread_prob: float = 0., micro_mod: float = 1., - callbacks: list[callable] | None = None, ) -> None: """Create a new edge between two nodes. @@ -236,10 +234,6 @@ def __init__( probability in case of only a microscopic node involvement. """ self.trigger_callbacks = [] - if callbacks is not None: - self.trigger_callbacks += callbacks - - self.parent: Tumor | LymphNodeLevel = parent self.child: LymphNodeLevel = child if ( @@ -353,7 +347,6 @@ def get_micro_mod(self) -> float: self._micro_mod = 1. return self._micro_mod - @trigger def set_micro_mod(self, new_micro_mod: float | None) -> None: """Set the spread modifier for LNLs with microscopic involvement.""" if new_micro_mod is None: @@ -380,7 +373,6 @@ def get_spread_prob(self) -> float: self._spread_prob = 0. return self._spread_prob - @trigger def set_spread_prob(self, new_spread_prob: float | None) -> None: """Set the spread probability of the edge.""" if new_spread_prob is None: @@ -493,7 +485,6 @@ def __init__( graph_dict: dict[tuple[str], list[str]], tumor_state: int | None = None, allowed_states: list[int] | None = None, - on_edge_change: list[callable] | None = None, ) -> None: """Create a new graph representation of nodes and edges. @@ -512,7 +503,7 @@ def __init__( check_unique_names(graph_dict) self._init_nodes(graph_dict, tumor_state, allowed_states) - self._init_edges(graph_dict, on_edge_change) + self._init_edges(graph_dict) def _init_nodes(self, graph, tumor_state, allowed_lnl_states): @@ -585,7 +576,6 @@ def is_trinary(self) -> bool: def _init_edges( self, graph: dict[tuple[str, str], list[str]], - on_edge_change: list[callable] ) -> None: """Initialize the edges of the ``graph``. @@ -602,12 +592,12 @@ def _init_edges( for (_, start_name), end_names in graph.items(): start = self.nodes[start_name] if isinstance(start, LymphNodeLevel) and start.is_trinary: - growth_edge = Edge(parent=start, child=start, callbacks=on_edge_change) + growth_edge = Edge(parent=start, child=start) self._edges[growth_edge.get_name()] = growth_edge for end_name in end_names: end = self.nodes[end_name] - new_edge = Edge(parent=start, child=end, callbacks=on_edge_change) + new_edge = Edge(parent=start, child=end) self._edges[new_edge.get_name()] = new_edge diff --git a/tests/edge_test.py b/tests/edge_test.py index bb09155..482afe6 100644 --- a/tests/edge_test.py +++ b/tests/edge_test.py @@ -13,12 +13,7 @@ def setUp(self) -> None: super().setUp() parent = graph.LymphNodeLevel("parent") child = graph.LymphNodeLevel("child") - self.was_called = False - self.edge = graph.Edge(parent, child, callbacks=[self.callback]) - - def callback(self) -> None: - """Callback function for the edge.""" - self.was_called = True + self.edge = graph.Edge(parent, child) def test_str(self) -> None: """Test the string representation of the edge.""" @@ -41,17 +36,11 @@ def test_repr(self) -> None: self.assertEqual(self.edge.spread_prob, recreated_edge.spread_prob) self.assertEqual(self.edge.micro_mod, recreated_edge.micro_mod) - def test_callback_on_param_change(self) -> None: - """Test if the callback function is called.""" - self.edge.spread_prob = 0.5 - self.assertTrue(self.was_called) - def test_graph_change(self) -> None: """Check if the callback also works when parent/child nodes are changed.""" old_child = self.edge.child new_child = graph.LymphNodeLevel("new_child") self.edge.child = new_child - self.assertTrue(self.was_called) self.assertNotIn(self.edge, old_child.inc) def test_transition_tensor_row_sums(self) -> None: diff --git a/tests/graph_representation_test.py b/tests/graph_representation_test.py index 5eb55df..4a8010c 100644 --- a/tests/graph_representation_test.py +++ b/tests/graph_representation_test.py @@ -34,15 +34,10 @@ def setUp(self) -> None: self.graph_repr = graph.Representation( graph_dict=self.graph_dict, allowed_states=[0, 1], - on_edge_change=[self.callback], ) self.was_called = False self.rng = np.random.default_rng(42) - def callback(self) -> None: - """Callback function for the graph.""" - self.was_called = True - def test_nodes(self) -> None: """Test the number of nodes.""" self.assertEqual(len(self.graph_repr.nodes), len(self.graph_dict)) From daabc27c0cd26d60885a0641b8cedae1817a8793 Mon Sep 17 00:00:00 2001 From: Roman Ludwig <48687784+rmnldwg@users.noreply.github.com> Date: Mon, 8 Apr 2024 17:35:49 +0200 Subject: [PATCH 5/9] fix(graph): wrong dict when trinary The `to_dict()` method returned a wrong graph dictionary when trinary due to growth edges. This is fixed now. --- lymph/graph.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lymph/graph.py b/lymph/graph.py index 3e92723..b943aad 100644 --- a/lymph/graph.py +++ b/lymph/graph.py @@ -659,7 +659,11 @@ def to_dict(self) -> dict[tuple[str, str], set[str]]: res = {} for node in self.nodes.values(): node_type = "tumor" if isinstance(node, Tumor) else "lnl" - res[(node_type, node.name)] = [o.child.name for o in node.out] + res[(node_type, node.name)] = [ + o.child.name + for o in node.out + if not o.is_growth + ] return res From 2fd112f621e41e2c9083217987f0a5b8b0745a89 Mon Sep 17 00:00:00 2001 From: Roman Ludwig <48687784+rmnldwg@users.noreply.github.com> Date: Mon, 8 Apr 2024 17:37:08 +0200 Subject: [PATCH 6/9] feat(graph): modify mermaid graph The `get_mermaid()` and `get_mermaid_url()` methods now accept arguments that allow some modifications of the output. --- lymph/graph.py | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/lymph/graph.py b/lymph/graph.py index b943aad..a21a88e 100644 --- a/lymph/graph.py +++ b/lymph/graph.py @@ -14,9 +14,11 @@ import base64 import warnings from itertools import product +from typing import Literal import numpy as np +from lymph import types from lymph.utils import ( check_unique_names, comp_transition_tensor, @@ -233,7 +235,7 @@ def __init__( spread to the next LNL. The ``micro_mod`` parameter is a modifier for the spread probability in case of only a microscopic node involvement. """ - self.trigger_callbacks = [] + self.parent: Tumor | LymphNodeLevel = parent self.child: LymphNodeLevel = child if ( @@ -667,7 +669,11 @@ def to_dict(self) -> dict[tuple[str, str], set[str]]: return res - def get_mermaid(self) -> str: + def get_mermaid( + self, + with_params: bool = True, + direction: Literal["TD", "LR"] = "TD", + ) -> str: """Prints the graph in mermaid format. >>> graph_dict = { @@ -685,19 +691,29 @@ def get_mermaid(self) -> str: T-->|20%| III II-->|30%| III + >>> print(graph.get_mermaid(with_params=False)) # doctest: +NORMALIZE_WHITESPACE + flowchart TD + T--> II + T--> III + II--> III + """ - mermaid_graph = "flowchart TD\n" + mermaid_graph = f"flowchart {direction}\n" for node in self.nodes.values(): for edge in node.out: - mermaid_graph += f"\t{node.name}-->|{edge.spread_prob:.0%}| {edge.child.name}\n" + param_str = f"|{edge.spread_prob:.0%}|" if with_params else "" + mermaid_graph += f"\t{node.name}-->{param_str} {edge.child.name}\n" return mermaid_graph - def get_mermaid_url(self) -> str: - """Returns the URL to the rendered graph.""" - mermaid_graph = self.get_mermaid() + def get_mermaid_url(self, **mermaid_kwargs) -> str: + """Returns the URL to the rendered graph. + + Keyword arguments are passed to :py:meth:`~Representation.get_mermaid`. + """ + mermaid_graph = self.get_mermaid(**mermaid_kwargs) graphbytes = mermaid_graph.encode("ascii") base64_bytes = base64.b64encode(graphbytes) base64_string = base64_bytes.decode("ascii") From f44d0343cab0ead0b6056ce0849bac373a082c28 Mon Sep 17 00:00:00 2001 From: Roman Ludwig <48687784+rmnldwg@users.noreply.github.com> Date: Mon, 8 Apr 2024 17:37:28 +0200 Subject: [PATCH 7/9] feat(uni): add `__repr__()` --- lymph/models/unilateral.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lymph/models/unilateral.py b/lymph/models/unilateral.py index f2cc40c..60fcbc8 100644 --- a/lymph/models/unilateral.py +++ b/lymph/models/unilateral.py @@ -117,6 +117,17 @@ def trinary(cls, graph_dict: types.GraphDictType, **kwargs) -> Unilateral: return cls(graph_dict, allowed_states=[0, 1, 2], **kwargs) + def __repr__(self) -> str: + """Return a string representation of the instance.""" + return ( + f"{type(self).__name__}(" + f"graph_dict={self.graph.to_dict()}, " + f"tumor_state={list(self.graph.tumors.values())[0].state}, " + f"allowed_states={self.graph.allowed_states}, " + f"max_time={self.max_time})" + ) + + def __str__(self) -> str: """Print info about the instance.""" return f"Unilateral with {len(self.graph.tumors)} tumors and {len(self.graph.lnls)} LNLs" From cd82d54d5e05136312518c2edd71bbdbf586d605 Mon Sep 17 00:00:00 2001 From: Roman Ludwig <48687784+rmnldwg@users.noreply.github.com> Date: Wed, 24 Apr 2024 18:06:01 +0200 Subject: [PATCH 8/9] fix: skip `marginalize` only when safe The marginalization should only be skipped (and 1 returned), when the entire disease state of interest is `None`. In the midline case, this disease state includes the midline extension. Previously, only the involvement pattern was checked. Now, the model is more careful about when to take shortcuts. --- lymph/models/bilateral.py | 2 +- lymph/models/midline.py | 4 ++-- lymph/models/unilateral.py | 6 +++++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lymph/models/bilateral.py b/lymph/models/bilateral.py index eef5174..2ecaa15 100644 --- a/lymph/models/bilateral.py +++ b/lymph/models/bilateral.py @@ -626,7 +626,7 @@ def marginalize( are ignored if ``given_state_dist`` is provided. """ if involvement is None: - return 1. + involvement = {} if given_state_dist is None: given_state_dist = self.state_dist(t_stage=t_stage, mode=mode) diff --git a/lymph/models/midline.py b/lymph/models/midline.py index f95c40a..74bc43b 100644 --- a/lymph/models/midline.py +++ b/lymph/models/midline.py @@ -770,7 +770,7 @@ def marginalize( :py:meth:`.state_dist` method. """ if involvement is None: - return 1. + involvement = {} if given_state_dist is None: given_state_dist = self.state_dist(t_stage=t_stage, mode=mode, central=central) @@ -787,7 +787,7 @@ def marginalize( given_state_dist = given_state_dist[int(midext)] # I think I don't need to normalize here, since I am not computing a # probability of something *given* midext, but only sum up all states that - # match the involvement pattern (which includes the midext status). + # match the disease state of interest (which includes the midext status). return self.ext.marginalize( involvement=involvement, diff --git a/lymph/models/unilateral.py b/lymph/models/unilateral.py index 60fcbc8..56644fd 100644 --- a/lymph/models/unilateral.py +++ b/lymph/models/unilateral.py @@ -835,7 +835,11 @@ def marginalize( :py:meth:`.state_dist` with the given ``t_stage`` and ``mode``. These arguments are ignored if ``given_state_dist`` is provided. """ - if involvement is None: + if ( + involvement is None + or not involvement # empty dict is falsey + or all(value is None for value in involvement.values()) + ): return 1. if given_state_dist is None: From dba519ad1731a9036c427dfa3aa7c9825b2e4b7f Mon Sep 17 00:00:00 2001 From: Roman Ludwig <48687784+rmnldwg@users.noreply.github.com> Date: Tue, 28 May 2024 15:14:01 +0200 Subject: [PATCH 9/9] chore: update changelog --- CHANGELOG.md | 47 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bcc8030..95b8748 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,50 @@ All notable changes to this project will be documented in this file. + + +## [1.2.1] - 2024-05-28 + +### Bug Fixes + +- (**uni**) `load_patient_data` should accept `None`. +- (**mid**) Correct type hint of `marginalize`. +- (**graph**) Wrong dict when trinary.\ + The `to_dict()` method returned a wrong graph dictionary when trinary + due to growth edges. This is fixed now. +- Skip `marginalize` only when safe.\ + The marginalization should only be skipped (and 1 returned), when the + entire disease state of interest is `None`. In the midline case, this + disease state includes the midline extension.\ + Previously, only the involvement pattern was checked. Now, the model is + more careful about when to take shortcuts. + + +### Features + +- (**graph**) Modify mermaid graph.\ + The `get_mermaid()` and `get_mermaid_url()` methods now accept arguments + that allow some modifications of the output. +- (**uni**) Add `__repr__()`. + +### Refactor + +- (**uni**) Use pandas `map` instead of `apply`.\ + This saves us a couple of lines in the `load_patient_data` method and is + more readable. + + +### Merge + +- Branch 'main' into 'dev'. + +### Remove + +- Remains of callbacks.\ + Some callback functionality that was tested in a pre-release has been + forgotten in the code base and is now deleted. + + ## [1.2.0] - 2024-03-29 @@ -668,7 +712,8 @@ Almost the entire API has changed. I'd therefore recommend to have a look at the - add pre-commit hook to check commit msg -[Unreleased]: https://github.com/rmnldwg/lymph/compare/1.2.0...HEAD +[Unreleased]: https://github.com/rmnldwg/lymph/compare/1.2.1...HEAD +[1.2.1]: https://github.com/rmnldwg/lymph/compare/1.1.0...1.2.1 [1.2.0]: https://github.com/rmnldwg/lymph/compare/1.1.0...1.2.0 [1.1.0]: https://github.com/rmnldwg/lymph/compare/1.0.0...1.1.0 [1.0.0]: https://github.com/rmnldwg/lymph/compare/1.0.0.rc2...1.0.0