YosefLab / Cassiopeia

A Package for Cas9-Enabled Single Cell Lineage Tracing Tree Reconstruction
https://cassiopeia-lineage.readthedocs.io/en/latest/
MIT License
75 stars 24 forks source link

RF and triplets correct should deepcopy #202

Closed sprillo closed 1 year ago

sprillo commented 1 year ago

A CassiopeiaTree is a complex nested object and should always be deepcopied, not shallow copied. In robinson_founds and triplets_correct, shallow copies of the input trees are being created before collapsing unifurcations, which as a result modifies the input trees:

https://github.com/YosefLab/Cassiopeia/blame/master/cassiopeia/critique/compare.py#L56-L61

Example:

import networkx as nx
import cassiopeia

tree_nx = nx.DiGraph()
tree_nx.add_nodes_from(["0", "1", "2", "3"]),
tree_nx.add_edges_from(
    [
        ("0", "1"),
        ("1", "2"),
        ("1", "3"),
    ]
)
tree = cassiopeia.data.CassiopeiaTree(tree=tree_nx)
print(f"Tree before computing RF: {tree.get_newick()}")
cassiopeia.critique.compare.robinson_foulds(tree, tree)
print(f"Tree after computing RF: {tree.get_newick()}")

Output:

Tree before computing RF: ((2,3));
Tree after computing RF: (2,3);

Expected output:

Tree before computing RF: ((2,3));
Tree after computing RF: ((2,3));

Bug seems to have been introduced 2 years ago during a refactor of the critique module:

https://github.com/YosefLab/Cassiopeia/commit/27d165379ae5ac904891affb0e78fe6840e428c3

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.01 :warning:

Comparison is base (e7606af) 79.05% compared to head (653e2b1) 79.05%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #202 +/- ## ========================================== - Coverage 79.05% 79.05% -0.01% ========================================== Files 85 85 Lines 7081 7080 -1 ========================================== - Hits 5598 5597 -1 Misses 1483 1483 ``` | [Impacted Files](https://app.codecov.io/gh/YosefLab/Cassiopeia/pull/202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=YosefLab) | Coverage Δ | | |---|---|---| | [cassiopeia/tools/fitness\_estimator/\_lbi\_jungle.py](https://app.codecov.io/gh/YosefLab/Cassiopeia/pull/202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=YosefLab#diff-Y2Fzc2lvcGVpYS90b29scy9maXRuZXNzX2VzdGltYXRvci9fbGJpX2p1bmdsZS5weQ==) | `94.11% <ø> (-0.12%)` | :arrow_down: | | [cassiopeia/critique/compare.py](https://app.codecov.io/gh/YosefLab/Cassiopeia/pull/202?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=YosefLab#diff-Y2Fzc2lvcGVpYS9jcml0aXF1ZS9jb21wYXJlLnB5) | `100.00% <100.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.