From 821bee430a59adc23b93b286d7cda1b606d7f80a Mon Sep 17 00:00:00 2001 From: Rob Letzler Date: Wed, 8 Jan 2025 23:23:29 -0500 Subject: [PATCH 1/6] code fix --- branca/element.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/branca/element.py b/branca/element.py index a1e005a..e2d2a3a 100644 --- a/branca/element.py +++ b/branca/element.py @@ -11,6 +11,7 @@ import warnings from binascii import hexlify from collections import OrderedDict +from copy import copy from html import escape from os import urandom from pathlib import Path @@ -69,6 +70,9 @@ def __init__( elif template_name is not None: self._template = ENV.get_template(template_name) + + + def __getstate__(self) -> dict: """Modify object state when pickling the object. @@ -88,6 +92,12 @@ def __setstate__(self, state: dict): self.__dict__.update(state) + def clone(): + """creates a new copy of an element, but with a unique identifier""" + clone = copy(self) + clone._id = hexlify(urandom(16)).decode() + return clone + def get_name(self) -> str: """Returns a string representation of the object. This string has to be unique and to be a python and @@ -143,6 +153,23 @@ def add_child( index: Optional[int] = None, ) -> "Element": """Add a child.""" + + # replace the proposed child with a clone of the proposed child because + # add_child 1) overwrites any existing value in the child._parent field + # and 2) does not do anything to remove the child from the list of children of the prior parent + # leading to inconsistency and to problems like the fact that adding the same icon to a map twice fails, as + # documented in https://github.com/python-visualization/folium/issues/1885 + # Creating a clone leads to internally consistent behavior if the + # child already has a parent, although existing code of the form: + # my_map.add_child(myElement) + # my_other_map.add_child(myElement) + # myElement.change + # will now have surprising new behavior + + if child._parent is not None: + child = child.clone() + + if name is None: name = child.get_name() if index is None: @@ -152,6 +179,7 @@ def add_child( items.insert(int(index), (name, child)) self._children = OrderedDict(items) child._parent = self + assert child._parent is not None return self def add_to( From fa675933fa8379efe21863e66b59f3ccccaf6e70 Mon Sep 17 00:00:00 2001 From: Rob Letzler Date: Tue, 21 Jan 2025 00:20:36 -0500 Subject: [PATCH 2/6] minor cleanup --- branca/element.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/branca/element.py b/branca/element.py index e2d2a3a..7c6c699 100644 --- a/branca/element.py +++ b/branca/element.py @@ -92,7 +92,7 @@ def __setstate__(self, state: dict): self.__dict__.update(state) - def clone(): + def clone(self): """creates a new copy of an element, but with a unique identifier""" clone = copy(self) clone._id = hexlify(urandom(16)).decode() @@ -163,10 +163,11 @@ def add_child( # child already has a parent, although existing code of the form: # my_map.add_child(myElement) # my_other_map.add_child(myElement) - # myElement.change + # myElement.change() # will now have surprising new behavior if child._parent is not None: + print("Note: the branca add_child function cloned an element rather than overwrite its existing parent element. This is new behavior as of early 2025. If you got this after issuing a command like my_map.add_child(myElement) and plan to issue a subsequent command like myElement.change(), that subsequent command will not affect the clone. Try either issuing the myElement.change() command first or creating a fresh version of myElement with no parent.") child = child.clone() From 7b5b1bf7e4f36b6f1a3a4fcc1cfcf2a040ce4ec4 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 21 Jan 2025 05:29:34 +0000 Subject: [PATCH 3/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- branca/element.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/branca/element.py b/branca/element.py index e2d2a3a..7fbc414 100644 --- a/branca/element.py +++ b/branca/element.py @@ -70,9 +70,6 @@ def __init__( elif template_name is not None: self._template = ENV.get_template(template_name) - - - def __getstate__(self) -> dict: """Modify object state when pickling the object. @@ -157,7 +154,7 @@ def add_child( # replace the proposed child with a clone of the proposed child because # add_child 1) overwrites any existing value in the child._parent field # and 2) does not do anything to remove the child from the list of children of the prior parent - # leading to inconsistency and to problems like the fact that adding the same icon to a map twice fails, as + # leading to inconsistency and to problems like the fact that adding the same icon to a map twice fails, as # documented in https://github.com/python-visualization/folium/issues/1885 # Creating a clone leads to internally consistent behavior if the # child already has a parent, although existing code of the form: @@ -169,7 +166,6 @@ def add_child( if child._parent is not None: child = child.clone() - if name is None: name = child.get_name() if index is None: From 1a94689c85d600e82d4cbd2846cfe4e5234627f9 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 21 Jan 2025 05:41:20 +0000 Subject: [PATCH 4/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- branca/element.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/branca/element.py b/branca/element.py index 485fbf0..343fe01 100644 --- a/branca/element.py +++ b/branca/element.py @@ -164,7 +164,9 @@ def add_child( # will now have surprising new behavior if child._parent is not None: - print("Note: the branca add_child function cloned an element rather than overwrite its existing parent element. This is new behavior as of early 2025. If you got this after issuing a command like my_map.add_child(myElement) and plan to issue a subsequent command like myElement.change(), that subsequent command will not affect the clone. Try either issuing the myElement.change() command first or creating a fresh version of myElement with no parent.") + print( + "Note: the branca add_child function cloned an element rather than overwrite its existing parent element. This is new behavior as of early 2025. If you got this after issuing a command like my_map.add_child(myElement) and plan to issue a subsequent command like myElement.change(), that subsequent command will not affect the clone. Try either issuing the myElement.change() command first or creating a fresh version of myElement with no parent.", + ) child = child.clone() if name is None: From ee407ccf6f1016cd312670dbf1b5bf40452c8251 Mon Sep 17 00:00:00 2001 From: Rob Letzler Date: Mon, 3 Feb 2025 19:01:13 -0500 Subject: [PATCH 5/6] changed from cloning to issuing a warning --- branca/element.py | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/branca/element.py b/branca/element.py index 485fbf0..1a8550c 100644 --- a/branca/element.py +++ b/branca/element.py @@ -90,9 +90,11 @@ def __setstate__(self, state: dict): self.__dict__.update(state) def clone(self): - """creates a new copy of an element, but with a unique identifier""" + """creates a new copy of an element, but with a unique identifier + and without the prior version's relationship to a parent object """ clone = copy(self) clone._id = hexlify(urandom(16)).decode() + clone._parent = None return clone def get_name(self) -> str: @@ -149,24 +151,26 @@ def add_child( name: Optional[str] = None, index: Optional[int] = None, ) -> "Element": - """Add a child.""" + """Add a child and modify the child with a pointer to its parent.""" - # replace the proposed child with a clone of the proposed child because - # add_child 1) overwrites any existing value in the child._parent field - # and 2) does not do anything to remove the child from the list of children of the prior parent - # leading to inconsistency and to problems like the fact that adding the same icon to a map twice fails, as - # documented in https://github.com/python-visualization/folium/issues/1885 - # Creating a clone leads to internally consistent behavior if the - # child already has a parent, although existing code of the form: - # my_map.add_child(myElement) - # my_other_map.add_child(myElement) - # myElement.change() - # will now have surprising new behavior if child._parent is not None: - print("Note: the branca add_child function cloned an element rather than overwrite its existing parent element. This is new behavior as of early 2025. If you got this after issuing a command like my_map.add_child(myElement) and plan to issue a subsequent command like myElement.change(), that subsequent command will not affect the clone. Try either issuing the myElement.change() command first or creating a fresh version of myElement with no parent.") - child = child.clone() - + warnings.warn( + f"""The code added\n {child.get_name()} as a child of + {self.get_name()} which + overwrote an existing pointer to a parent object, {child._parent.get_name()}, + but leaves the parent object pointing to the child. + Branca is designed so each object will have no more than + one parent. Consider replacing the object with object.clone() + so that each object has only one parent. Otherwise, actions may + fail. For example, adding an icon object to a map + only adds one valid icon to the map regardless of how many times + the code adds that icon object. This behavior is documented at: + https://github.com/python-visualization/folium/issues/1885 + """, + category=UserWarning, + stacklevel=2 + ) if name is None: name = child.get_name() if index is None: @@ -176,7 +180,6 @@ def add_child( items.insert(int(index), (name, child)) self._children = OrderedDict(items) child._parent = self - assert child._parent is not None return self def add_to( From 47edfb62eaad644ffbe804c9490c272474d90f36 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 4 Feb 2025 00:11:46 +0000 Subject: [PATCH 6/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- branca/element.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/branca/element.py b/branca/element.py index 1a8550c..9b68033 100644 --- a/branca/element.py +++ b/branca/element.py @@ -90,8 +90,8 @@ def __setstate__(self, state: dict): self.__dict__.update(state) def clone(self): - """creates a new copy of an element, but with a unique identifier - and without the prior version's relationship to a parent object """ + """creates a new copy of an element, but with a unique identifier + and without the prior version's relationship to a parent object""" clone = copy(self) clone._id = hexlify(urandom(16)).decode() clone._parent = None @@ -153,24 +153,23 @@ def add_child( ) -> "Element": """Add a child and modify the child with a pointer to its parent.""" - if child._parent is not None: warnings.warn( - f"""The code added\n {child.get_name()} as a child of - {self.get_name()} which + f"""The code added\n {child.get_name()} as a child of + {self.get_name()} which overwrote an existing pointer to a parent object, {child._parent.get_name()}, - but leaves the parent object pointing to the child. + but leaves the parent object pointing to the child. Branca is designed so each object will have no more than - one parent. Consider replacing the object with object.clone() + one parent. Consider replacing the object with object.clone() so that each object has only one parent. Otherwise, actions may fail. For example, adding an icon object to a map - only adds one valid icon to the map regardless of how many times + only adds one valid icon to the map regardless of how many times the code adds that icon object. This behavior is documented at: https://github.com/python-visualization/folium/issues/1885 """, - category=UserWarning, - stacklevel=2 - ) + category=UserWarning, + stacklevel=2, + ) if name is None: name = child.get_name() if index is None: