Merge pull request #308 from kayjan/bugfix/tree-diff

Fix get_tree_diff

Author: kayjan

CHANGELOG.md

@@ -6,6 +6,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.21.3] - 2024-10-16
+### Added:
+- Tree Node: Docstring indentation and additional information for Node creation.
+- Misc: GitHub star diagram to README file.
+### Changed:
+- Tree Helper: Get tree diff to handle `sep` that are different for tree and other_tree, this will throw error now.
+- Tree Helper: Get tree diff to handle `sep` that contains forbidden symbols, this will change the `sep` to a fallback sep.
+- Tree Helper: Get tree diff to sort tree before returning the tree diff.
+### Fixed:
+- [#306] Tree Helper: Get tree diff to handle `sep` that is different from default.
+
 ## [0.21.2] - 2024-10-14
 ### Added:
 - Misc: Pull request template.
@@ -657,7 +668,8 @@ ignore null attribute columns.
 - Utility Iterator: Tree traversal methods.
 - Workflow To Do App: Tree use case with to-do list implementation.
 
-[Unreleased]: https://github.com/kayjan/bigtree/compare/0.21.2...HEAD
+[Unreleased]: https://github.com/kayjan/bigtree/compare/0.21.3...HEAD
+[0.21.3]: https://github.com/kayjan/bigtree/compare/0.21.2...0.21.3
 [0.21.2]: https://github.com/kayjan/bigtree/compare/0.21.1...0.21.2
 [0.21.1]: https://github.com/kayjan/bigtree/compare/0.21.0...0.21.1
 [0.21.0]: https://github.com/kayjan/bigtree/compare/0.20.1...0.21.0

README.md

@@ -151,3 +151,9 @@ To install `bigtree` with conda, run the following line in command prompt:
 ```console
 $ conda install -c conda-forge bigtree
 ```
+
+-----
+
+## Star History
+
+[![Star History Chart](https://api.star-history.com/svg?repos=kayjan/bigtree&type=Date)](https://star-history.com/#kayjan/bigtree&Date)

bigtree/__init__.py

@@ -1,4 +1,4 @@
-__version__ = "0.21.2"
+__version__ = "0.21.3"
 
 from bigtree.binarytree.construct import list_to_binarytree
 from bigtree.dag.construct import dataframe_to_dag, dict_to_dag, list_to_dag

bigtree/tree/helper.py

@@ -242,6 +242,7 @@ def get_tree_diff(
     other_tree: node.Node,
     only_diff: bool = True,
     attr_list: List[str] = [],
+    fallback_sep: str = "/",
 ) -> node.Node:
     """Get difference of `tree` to `other_tree`, changes are relative to `tree`.
 
@@ -254,6 +255,12 @@ def get_tree_diff(
     - For example: (+) refers to nodes that are in `other_tree` but not `tree`.
     - For example: (-) refers to nodes that are in `tree` but not `other_tree`.
 
+    !!! note
+
+        - tree and other_tree must have the same `sep` symbol, otherwise this will raise ValueError
+        - If the `sep` symbol contains one of `+` / `-` / `~` character, a fallback sep will be used
+        - Node names in tree and other_tree must not contain the `sep` (or fallback sep) symbol
+
     Examples:
         >>> # Create original tree
         >>> from bigtree import Node, get_tree_diff, list_to_tree
@@ -333,11 +340,25 @@ def get_tree_diff(
         other_tree (Node): tree to be compared with
         only_diff (bool): indicator to show all nodes or only nodes that are different (+/-), defaults to True
         attr_list (List[str]): tree attributes to check for difference, defaults to empty list
+        fallback_sep (str): sep to fall back to if tree and other_tree has sep that clashes with symbols "+" / "-" / "~".
+            All node names in tree and other_tree should not contain this fallback_sep, defaults to "/"
 
     Returns:
         (Node)
     """
-    other_tree.sep = tree.sep
+    if tree.sep != other_tree.sep:
+        raise ValueError("`sep` must be the same for tree and other_tree")
+
+    forbidden_sep_symbols = ["+", "-", "~"]
+    if any(
+        forbidden_sep_symbol in tree.sep
+        for forbidden_sep_symbol in forbidden_sep_symbols
+    ):
+        tree = tree.copy()
+        other_tree = other_tree.copy()
+        tree.sep = fallback_sep
+        other_tree.sep = fallback_sep
+
     name_col = "name"
     path_col = "PATH"
     indicator_col = "Exists"
@@ -403,9 +424,11 @@ def get_tree_diff(
             (data_both[indicator_col] != "both")
             | (data_both[path_col].isin(path_changes_deque))
         ]
-    data_both = data_both[[path_col]]
+    data_both = data_both[[path_col]].sort_values(path_col)
     if len(data_both):
-        tree_diff = construct.dataframe_to_tree(data_both, node_type=tree.__class__)
+        tree_diff = construct.dataframe_to_tree(
+            data_both, node_type=tree.__class__, sep=tree.sep
+        )
         # Handle tree attribute difference
         if len(path_changes_deque):
             path_changes_list = sorted(path_changes_deque, reverse=True)

tests/test_constants.py

@@ -169,6 +169,7 @@ class Constants:
     ERROR_NODE_PRUNE_NOT_FOUND = (
         "Cannot find any node matching path_name ending with {prune_path}"
     )
+    ERROR_NODE_TREE_DIFF_DIFF_SEP = "`sep` must be the same for tree and other_tree"
 
     # tree/modify
     ERROR_MODIFY_PARAM_TYPE = (

tests/tree/test_helper.py

@@ -236,22 +236,58 @@ def test_tree_diff(tree_node):
         assert_print_statement(export.print_tree, expected_str, tree=tree_only_diff)
 
     @staticmethod
-    def test_tree_diff_diff_sep(tree_node):
+    def test_tree_diff_diff_sep_error(tree_node):
         other_tree_node = helper.prune_tree(tree_node, "a/c")
-        _ = node.Node("d", parent=other_tree_node)
         other_tree_node.sep = "-"
+        with pytest.raises(ValueError) as exc_info:
+            helper.get_tree_diff(tree_node, other_tree_node)
+        assert str(exc_info.value) == Constants.ERROR_NODE_TREE_DIFF_DIFF_SEP
+
+    @staticmethod
+    def test_tree_diff_sep_clash_with_node_name_error(tree_node):
+        other_tree_node = helper.prune_tree(tree_node, "a/c")
+        _ = node.Node("/d", parent=other_tree_node)
+        with pytest.raises(exceptions.TreeError) as exc_info:
+            helper.get_tree_diff(tree_node, other_tree_node)
+        assert str(exc_info.value) == Constants.ERROR_NODE_NAME
+
+    @staticmethod
+    def test_tree_diff_sep_clash_with_node_name(tree_node):
+        other_tree_node = helper.prune_tree(tree_node, "a/c")
+        _ = node.Node("/d", parent=other_tree_node)
+        tree_node.sep = "."
+        other_tree_node.sep = "."
         tree_only_diff = helper.get_tree_diff(tree_node, other_tree_node)
         expected_str = (
             "a\n"
-            "├── b (-)\n"
-            "│   ├── d (-)\n"
-            "│   └── e (-)\n"
-            "│       ├── g (-)\n"
-            "│       └── h (-)\n"
-            "└── d (+)\n"
+            "├── /d (+)\n"
+            "└── b (-)\n"
+            "    ├── d (-)\n"
+            "    └── e (-)\n"
+            "        ├── g (-)\n"
+            "        └── h (-)\n"
         )
         assert_print_statement(export.print_tree, expected_str, tree=tree_only_diff)
 
+    @staticmethod
+    def test_tree_diff_forbidden_sep(tree_node):
+        other_tree_node = helper.prune_tree(tree_node, "a/c")
+        _ = node.Node("d", parent=other_tree_node)
+        for symbol in [".", "-", "+", "~"]:
+            tree_node.sep = symbol
+            other_tree_node.sep = symbol
+            tree_only_diff = helper.get_tree_diff(tree_node, other_tree_node)
+            expected_str = (
+                "a\n"
+                "├── b (-)\n"
+                "│   ├── d (-)\n"
+                "│   └── e (-)\n"
+                "│       ├── g (-)\n"
+                "│       └── h (-)\n"
+                "└── d (+)\n"
+            )
+            assert_print_statement(export.print_tree, expected_str, tree=tree_only_diff)
+
     @staticmethod
     def test_tree_diff_all_diff(tree_node):
         other_tree_node = helper.prune_tree(tree_node, "a/c")