caesar0301 / treelib

An efficient implementation of tree data structure in python 2/3.
http://treelib.readthedocs.io/en/latest/
Other
801 stars 185 forks source link

deepcopy of fpointers list #160

Open mguijarr opened 4 years ago

mguijarr commented 4 years ago

First of all, thanks a lot for this library : it is very useful !

Recently at my company we wanted to update to treelib 1.6.1, however we started to have problems because now everything stored in the tree has to be "picklable", because of the use of copy.deepcopy in the clone_pointers method.

Why using deepcopy, in this case ? 1) I think it is rather inefficient, 2) it really puts an additional constraint on the kind of object in the fpointer list, which was not there before

According to the comment in the corresponding code, this is done to not pass the fpointers list as a reference.

Then, would you consider using shallow copy instead of deepcopy ?

Proposal:

diff --git a/treelib/node.py b/treelib/node.py                                                                                                                               index 6accae1..910d2e2 100644                                                                                                                                                --- a/treelib/node.py                                                                                                                                                        +++ b/treelib/node.py                                                                                                                                                        @@ -217,7 +217,7 @@ class Node(object):                                                                                                                                      
         self.set_predecessor(former_bpointer, new_tree_id)                                                                                                                  
         former_fpointer = self.successors(former_tree_id)                                                                                                                   
         # fpointer is a list and would be copied by reference without deepcopy                                                                                              
-        self.set_successors(copy.deepcopy(former_fpointer), tree_id=new_tree_id)                                                                                            
+        self.set_successors(list(former_fpointer), tree_id=new_tree_id)                                                                                                     

     def reset_pointers(self, tree_id):                                                                                                                                      
         self.set_predecessor(None, tree_id)