rmnldwg / lymph

Python package for statistical modelling of lymphatic metastatic spread in head & neck cancer.
https://lymph-model.readthedocs.io
MIT License
5 stars 4 forks source link

`nodes` in `graph` not correctly accessed #64

Closed YoelPH closed 5 months ago

YoelPH commented 6 months ago

I found another small issue originating from the fact that graph.nodes is not an object anymore but a dictionary. In graph.py two functions: to_dict and get_mermaid both try to use self.nodes as an object:

    def to_dict(self) -> dict[tuple[str, str], set[str]]:
        """Returns graph representing this instance's nodes and egdes as dictionary."""
        res = {}
        for node in self.nodes:
            node_type = "tumor" if isinstance(node, Tumor) else "lnl"
            res[(node_type, node.name)] = {o.child.name for o in node.out}
        return res

    def get_mermaid(self) -> str:
        """Prints the graph in mermaid format.

        Example:

        >>> graph_dict = {
        ...    ("tumor", "T"): ["II", "III"],
        ...    ("lnl", "II"): ["III"],
        ...    ("lnl", "III"): [],
        ... }
        >>> graph = Representation(graph_dict)
        >>> graph.edge_params["spread_T_to_II"].set_param(0.1)
        >>> graph.edge_params["spread_T_to_III"].set_param(0.2)
        >>> graph.edge_params["spread_II_to_III"].set_param(0.3)
        >>> print(graph.get_mermaid())  # doctest: +NORMALIZE_WHITESPACE
        flowchart TD
            T-->|10%| II
            T-->|20%| III
            II-->|30%| III
        <BLANKLINE>
        """
        mermaid_graph = "flowchart TD\n"

        for idx, node in enumerate(self.nodes):
            for edge in self.nodes[idx].out:
                mermaid_graph += f"\t{node.name}-->|{edge.spread_prob:.0%}| {edge.child.name}\n"

        return mermaid_graph

This does not work anmymore. For get_mermaid for example we need to fix it like this:

        for idx, node in enumerate(self.nodes):
            for edge in self.nodes[node].out:
                mermaid_graph += f"\t{node}-->|{edge.spread_prob:.0%}| {edge.child.name}\n"

        return mermaid_graph

(idx would not be needed anymore)

rmnldwg commented 6 months ago

Thanks for the catch! The issue should be fixed on the dev branch now 👍🏻

With that issue, I noticed that the automatic test discovery does not actually run the doctests. I fixed that too, which is nice.