From becda03b7391c011f9d7228872d5f239c6b54c56 Mon Sep 17 00:00:00 2001 From: Hans Then Date: Sun, 26 Jan 2025 13:19:57 +0100 Subject: [PATCH 1/2] Cleanup macroelements Earlier I noted many macroelements do not fully use the facilities of Figure, but instead in render manipulate the parent Figure. Step 1: remove GlobalSwitches as Element --- folium/elements.py | 4 + folium/folium.py | 87 +++++--------- folium/map.py | 57 ++++----- folium/plugins/draw.py | 57 ++++----- folium/plugins/heat_map_withtime.py | 178 +++++++++++++--------------- folium/plugins/search.py | 19 +-- folium/raster_layers.py | 42 +++---- tests/test_folium.py | 20 ++-- tests/test_map.py | 12 +- 9 files changed, 199 insertions(+), 277 deletions(-) diff --git a/folium/elements.py b/folium/elements.py index 56b3ba9040..c99e35474b 100644 --- a/folium/elements.py +++ b/folium/elements.py @@ -18,6 +18,10 @@ class JSCSSMixin(MacroElement): default_js: List[Tuple[str, str]] = [] default_css: List[Tuple[str, str]] = [] + # Since this is typically used as a mixin, we cannot + # override the _template member variable here. It would + # be overwritten by any subclassing class that also has + # a _template variable. def render(self, **kwargs): figure = self.get_root() assert isinstance( diff --git a/folium/folium.py b/folium/folium.py index 12510ecb43..8c47084ddb 100644 --- a/folium/folium.py +++ b/folium/folium.py @@ -7,7 +7,7 @@ import webbrowser from typing import Any, List, Optional, Sequence, Union -from branca.element import Element, Figure +from branca.element import Figure from folium.elements import JSCSSMixin from folium.map import Evented, FitBounds, Layer @@ -62,23 +62,6 @@ ] -class GlobalSwitches(Element): - _template = Template( - """ - - """ - ) - - def __init__(self, no_touch=False, disable_3d=False): - super().__init__() - self._name = "GlobalSwitches" - self.no_touch = no_touch - self.disable_3d = disable_3d - - class Map(JSCSSMixin, Evented): """Create a Map with Folium and Leaflet.js @@ -192,6 +175,30 @@ class Map(JSCSSMixin, Evented): } .leaflet-container { font-size: {{this.font_size}}; } + + + + + + + + {% endmacro %} {% macro html(this, kwargs) %} @@ -304,6 +311,9 @@ def __init__( else: self.zoom_control_position = False + self.no_touch = no_touch + self.disable_3d = disable_3d + self.options = remove_empty( max_bounds=max_bounds_array, zoom=zoom_start, @@ -312,8 +322,6 @@ def __init__( **kwargs, ) - self.global_switches = GlobalSwitches(no_touch, disable_3d) - self.objects_to_stay_in_front: List[Layer] = [] if isinstance(tiles, TileLayer): @@ -377,45 +385,6 @@ def _repr_png_(self) -> Optional[bytes]: return None return self._to_png() - def render(self, **kwargs): - """Renders the HTML representation of the element.""" - figure = self.get_root() - assert isinstance( - figure, Figure - ), "You cannot render this Element if it is not in a Figure." - - # Set global switches - figure.header.add_child(self.global_switches, name="global_switches") - - figure.header.add_child( - Element( - "" - ), - name="css_style", - ) - - figure.header.add_child( - Element( - "" - ), - name="map_style", - ) - - super().render(**kwargs) - def show_in_browser(self) -> None: """Display the Map in the default web browser.""" with temp_html_filepath(self.get_root().render()) as fname: diff --git a/folium/map.py b/folium/map.py index e922906713..b10441cf03 100644 --- a/folium/map.py +++ b/folium/map.py @@ -7,7 +7,7 @@ from collections import OrderedDict from typing import TYPE_CHECKING, Optional, Sequence, Union, cast -from branca.element import Element, Figure, Html, MacroElement +from branca.element import Element, Html, MacroElement from folium.elements import ElementAddToElement, EventHandler from folium.template import Template @@ -454,25 +454,27 @@ class Popup(MacroElement): _template = Template( """ - var {{this.get_name()}} = L.popup({{ this.options|tojavascript }}); - - {% for name, element in this.html._children.items() %} - {% if this.lazy %} - {{ this._parent.get_name() }}.once('click', function() { - {{ this.get_name() }}.setContent($(`{{ element.render(**kwargs).replace('\\n',' ') }}`)[0]); - }); - {% else %} - var {{ name }} = $(`{{ element.render(**kwargs).replace('\\n',' ') }}`)[0]; - {{ this.get_name() }}.setContent({{ name }}); - {% endif %} - {% endfor %} - - {{ this._parent.get_name() }}.bindPopup({{ this.get_name() }}) - {% if this.show %}.openPopup(){% endif %}; - - {% for name, element in this.script._children.items() %} - {{element.render()}} - {% endfor %} + {% macro script(this, kwargs) %} + var {{this.get_name()}} = L.popup({{ this.options|tojavascript }}); + + {% for name, element in this.html._children.items() %} + {% if this.lazy %} + {{ this._parent.get_name() }}.once('click', function() { + {{ this.get_name() }}.setContent($(`{{ element.render(**kwargs).replace('\\n',' ') }}`)[0]); + }); + {% else %} + var {{ name }} = $(`{{ element.render(**kwargs).replace('\\n',' ') }}`)[0]; + {{ this.get_name() }}.setContent({{ name }}); + {% endif %} + {% endfor %} + + {{ this._parent.get_name() }}.bindPopup({{ this.get_name() }}) + {% if this.show %}.openPopup(){% endif %}; + + {% for name, element in this.script._children.items() %} + {{element.render()}} + {% endfor %} + {% endmacro %} """ ) # noqa @@ -513,21 +515,6 @@ def __init__( **kwargs, ) - def render(self, **kwargs): - """Renders the HTML representation of the element.""" - for name, child in self._children.items(): - child.render(**kwargs) - - figure = self.get_root() - assert isinstance( - figure, Figure - ), "You cannot render this Element if it is not in a Figure." - - figure.script.add_child( - Element(self._template.render(this=self, kwargs=kwargs)), - name=self.get_name(), - ) - class Tooltip(MacroElement): """ diff --git a/folium/plugins/draw.py b/folium/plugins/draw.py index b00311e084..9a614e6595 100644 --- a/folium/plugins/draw.py +++ b/folium/plugins/draw.py @@ -1,4 +1,4 @@ -from branca.element import Element, Figure, MacroElement +from branca.element import MacroElement from folium.elements import JSCSSMixin from folium.template import Template @@ -60,6 +60,29 @@ class Draw(JSCSSMixin, MacroElement): _template = Template( """ + {% macro html(this, kwargs) %} + {% if this.export %} + + Export + {% endif %} + {% endmacro %} + {% macro script(this, kwargs) %} var options = { position: {{ this.position|tojson }}, @@ -155,35 +178,3 @@ def __init__( self.draw_options = draw_options or {} self.edit_options = edit_options or {} self.on = on or {} - - def render(self, **kwargs): - super().render(**kwargs) - - figure = self.get_root() - assert isinstance( - figure, Figure - ), "You cannot render this Element if it is not in a Figure." - - export_style = """ - - """ - export_button = """Export""" - if self.export: - figure.header.add_child(Element(export_style), name="export") - figure.html.add_child(Element(export_button), name="export_button") diff --git a/folium/plugins/heat_map_withtime.py b/folium/plugins/heat_map_withtime.py index 1623133543..56ccb2599d 100644 --- a/folium/plugins/heat_map_withtime.py +++ b/folium/plugins/heat_map_withtime.py @@ -1,5 +1,3 @@ -from branca.element import Element, Figure - from folium.elements import JSCSSMixin from folium.map import Layer from folium.template import Template @@ -61,6 +59,87 @@ class HeatMapWithTime(JSCSSMixin, Layer): _template = Template( """ + {% macro header(this, kwargs) %} + + {% endmacro %} + {% macro script(this, kwargs) %} var times = {{this.times}}; @@ -202,101 +281,6 @@ def __init__( self.time_slider_drag_update = "false" self.style_NS = "leaflet-control-timecontrol" - def render(self, **kwargs): - super().render(**kwargs) - - figure = self.get_root() - assert isinstance( - figure, Figure - ), "You cannot render this Element if it is not in a Figure." - - figure.header.add_child( - Element( - """ - - """, # noqa - template_name="timeControlScript", - ) - ) - def _get_self_bounds(self): """ Computes the bounds of the object itself (not including it's children) diff --git a/folium/plugins/search.py b/folium/plugins/search.py index aa74e0f69b..c13b1af6ed 100644 --- a/folium/plugins/search.py +++ b/folium/plugins/search.py @@ -1,6 +1,5 @@ from branca.element import MacroElement -from folium import Map from folium.elements import JSCSSMixin from folium.features import FeatureGroup, GeoJson, TopoJson from folium.plugins import MarkerCluster @@ -123,17 +122,9 @@ def __init__( self.placeholder = placeholder self.collapsed = collapsed self.options = remove_empty(**kwargs) + self.test_params() - def test_params(self, keys): - if keys is not None and self.search_label is not None: - assert self.search_label in keys, ( - f"The label '{self.search_label}' was not " f"available in {keys}" "" - ) - assert isinstance( - self._parent, Map - ), "Search can only be added to folium Map objects." - - def render(self, **kwargs): + def test_params(self): if isinstance(self.layer, GeoJson): keys = tuple(self.layer.data["features"][0]["properties"].keys()) elif isinstance(self.layer, TopoJson): @@ -145,6 +136,8 @@ def render(self, **kwargs): ) # noqa else: keys = None - self.test_params(keys=keys) - super().render(**kwargs) + if keys is not None and self.search_label is not None: + assert self.search_label in keys, ( + f"The label '{self.search_label}' was not " f"available in {keys}" "" + ) diff --git a/folium/raster_layers.py b/folium/raster_layers.py index 0ff6ef9948..39a7de0e34 100644 --- a/folium/raster_layers.py +++ b/folium/raster_layers.py @@ -6,7 +6,6 @@ from typing import Any, Callable, Optional, Union import xyzservices -from branca.element import Element, Figure from folium.map import Layer from folium.template import Template @@ -285,6 +284,22 @@ class ImageOverlay(Layer): _template = Template( """ + {% macro header(this, kwargs) %} + {% if this.pixelated %} + + {% endif %} + {% endmacro %} + {% macro script(this, kwargs) %} var {{ this.get_name() }} = L.imageOverlay( {{ this.url|tojson }}, @@ -321,31 +336,6 @@ def __init__( self.url = image_to_url(image, origin=origin, colormap=colormap) - def render(self, **kwargs): - super().render() - - figure = self.get_root() - assert isinstance( - figure, Figure - ), "You cannot render this Element if it is not in a Figure." - if self.pixelated: - pixelated = """ - - """ - figure.header.add_child( - Element(pixelated), name="leaflet-image-layer" - ) # noqa - def _get_self_bounds(self) -> TypeBoundsReturn: """ Computes the bounds of the object itself (not including it's children) diff --git a/tests/test_folium.py b/tests/test_folium.py index 58ad28c978..96b09a06f8 100644 --- a/tests/test_folium.py +++ b/tests/test_folium.py @@ -98,8 +98,8 @@ def test_init(self): assert self.m.width == (900, "px") assert self.m.left == (0, "%") assert self.m.top == (0, "%") - assert self.m.global_switches.no_touch is False - assert self.m.global_switches.disable_3d is False + assert self.m.no_touch is False + assert self.m.disable_3d is False assert self.m.font_size == "1.5rem" assert self.m.to_dict() == { "name": "Map", @@ -465,29 +465,29 @@ def test_global_switches(self): out = m._parent.render() out_str = "".join(out.split()) assert '"preferCanvas":true' in out_str - assert not m.global_switches.no_touch - assert not m.global_switches.disable_3d + assert not m.no_touch + assert not m.disable_3d m = folium.Map(no_touch=True) out = m._parent.render() out_str = "".join(out.split()) assert '"preferCanvas":false' in out_str - assert m.global_switches.no_touch - assert not m.global_switches.disable_3d + assert m.no_touch + assert not m.disable_3d m = folium.Map(disable_3d=True) out = m._parent.render() out_str = "".join(out.split()) assert '"preferCanvas":false' in out_str - assert not m.global_switches.no_touch - assert m.global_switches.disable_3d + assert not m.no_touch + assert m.disable_3d m = folium.Map(prefer_canvas=True, no_touch=True, disable_3d=True) out = m._parent.render() out_str = "".join(out.split()) assert '"preferCanvas":true' in out_str - assert m.global_switches.no_touch - assert m.global_switches.disable_3d + assert m.no_touch + assert m.disable_3d def test_json_request(self): """Test requests for remote GeoJSON files.""" diff --git a/tests/test_map.py b/tests/test_map.py index cc3728586a..1e099f9b93 100644 --- a/tests/test_map.py +++ b/tests/test_map.py @@ -109,7 +109,8 @@ def test_popup_unicode(): def test_popup_sticky(): m = Map() popup = Popup("Some text.", sticky=True).add_to(m) - rendered = popup._template.render(this=popup, kwargs={}) + script = popup._template.module.__dict__.get("script", None) + rendered = script(this=popup, kwargs={}) expected = """ var {popup_name} = L.popup({{ "maxWidth": "100%", @@ -131,7 +132,8 @@ def test_popup_sticky(): def test_popup_show(): m = Map() popup = Popup("Some text.", show=True).add_to(m) - rendered = popup._template.render(this=popup, kwargs={}) + script = popup._template.module.__dict__.get("script", None) + rendered = script(this=popup, kwargs={}) expected = """ var {popup_name} = L.popup({{ "maxWidth": "100%","autoClose": false, @@ -151,7 +153,8 @@ def test_popup_show(): def test_popup_backticks(): m = Map() popup = Popup("back`tick`tick").add_to(m) - rendered = popup._template.render(this=popup, kwargs={}) + script = popup._template.module.__dict__.get("script", None) + rendered = script(this=popup, kwargs={}) expected = """ var {popup_name} = L.popup({{ "maxWidth": "100%", @@ -170,7 +173,8 @@ def test_popup_backticks(): def test_popup_backticks_already_escaped(): m = Map() popup = Popup("back\\`tick").add_to(m) - rendered = popup._template.render(this=popup, kwargs={}) + script = popup._template.module.__dict__.get("script", None) + rendered = script(this=popup, kwargs={}) expected = """ var {popup_name} = L.popup({{ "maxWidth": "100%", From 0e9ef121b11830b6e9349934062cc99c539b990b Mon Sep 17 00:00:00 2001 From: Hans Then Date: Sun, 16 Mar 2025 21:34:57 +0100 Subject: [PATCH 2/2] Based on review comments --- folium/folium.py | 27 ++++++++++++++----- folium/map.py | 57 ++++++++++++++++++++++++---------------- folium/plugins/search.py | 19 +++++++++----- tests/test_folium.py | 20 +++++++------- tests/test_map.py | 12 +++------ 5 files changed, 83 insertions(+), 52 deletions(-) diff --git a/folium/folium.py b/folium/folium.py index 8c47084ddb..c8dc8dc6e7 100644 --- a/folium/folium.py +++ b/folium/folium.py @@ -7,7 +7,7 @@ import webbrowser from typing import Any, List, Optional, Sequence, Union -from branca.element import Figure +from branca.element import Element, Figure from folium.elements import JSCSSMixin from folium.map import Evented, FitBounds, Layer @@ -62,6 +62,23 @@ ] +class GlobalSwitches(Element): + _template = Template( + """ + + """ + ) + + def __init__(self, no_touch=False, disable_3d=False): + super().__init__() + self._name = "GlobalSwitches" + self.no_touch = no_touch + self.disable_3d = disable_3d + + class Map(JSCSSMixin, Evented): """Create a Map with Folium and Leaflet.js @@ -193,10 +210,9 @@ class Map(JSCSSMixin, Evented): } - {% endmacro %} @@ -311,8 +327,7 @@ def __init__( else: self.zoom_control_position = False - self.no_touch = no_touch - self.disable_3d = disable_3d + self.global_switches = GlobalSwitches(no_touch, disable_3d) self.options = remove_empty( max_bounds=max_bounds_array, diff --git a/folium/map.py b/folium/map.py index b10441cf03..e922906713 100644 --- a/folium/map.py +++ b/folium/map.py @@ -7,7 +7,7 @@ from collections import OrderedDict from typing import TYPE_CHECKING, Optional, Sequence, Union, cast -from branca.element import Element, Html, MacroElement +from branca.element import Element, Figure, Html, MacroElement from folium.elements import ElementAddToElement, EventHandler from folium.template import Template @@ -454,27 +454,25 @@ class Popup(MacroElement): _template = Template( """ - {% macro script(this, kwargs) %} - var {{this.get_name()}} = L.popup({{ this.options|tojavascript }}); - - {% for name, element in this.html._children.items() %} - {% if this.lazy %} - {{ this._parent.get_name() }}.once('click', function() { - {{ this.get_name() }}.setContent($(`{{ element.render(**kwargs).replace('\\n',' ') }}`)[0]); - }); - {% else %} - var {{ name }} = $(`{{ element.render(**kwargs).replace('\\n',' ') }}`)[0]; - {{ this.get_name() }}.setContent({{ name }}); - {% endif %} - {% endfor %} - - {{ this._parent.get_name() }}.bindPopup({{ this.get_name() }}) - {% if this.show %}.openPopup(){% endif %}; - - {% for name, element in this.script._children.items() %} - {{element.render()}} - {% endfor %} - {% endmacro %} + var {{this.get_name()}} = L.popup({{ this.options|tojavascript }}); + + {% for name, element in this.html._children.items() %} + {% if this.lazy %} + {{ this._parent.get_name() }}.once('click', function() { + {{ this.get_name() }}.setContent($(`{{ element.render(**kwargs).replace('\\n',' ') }}`)[0]); + }); + {% else %} + var {{ name }} = $(`{{ element.render(**kwargs).replace('\\n',' ') }}`)[0]; + {{ this.get_name() }}.setContent({{ name }}); + {% endif %} + {% endfor %} + + {{ this._parent.get_name() }}.bindPopup({{ this.get_name() }}) + {% if this.show %}.openPopup(){% endif %}; + + {% for name, element in this.script._children.items() %} + {{element.render()}} + {% endfor %} """ ) # noqa @@ -515,6 +513,21 @@ def __init__( **kwargs, ) + def render(self, **kwargs): + """Renders the HTML representation of the element.""" + for name, child in self._children.items(): + child.render(**kwargs) + + figure = self.get_root() + assert isinstance( + figure, Figure + ), "You cannot render this Element if it is not in a Figure." + + figure.script.add_child( + Element(self._template.render(this=self, kwargs=kwargs)), + name=self.get_name(), + ) + class Tooltip(MacroElement): """ diff --git a/folium/plugins/search.py b/folium/plugins/search.py index c13b1af6ed..ee7bd6bdb5 100644 --- a/folium/plugins/search.py +++ b/folium/plugins/search.py @@ -2,6 +2,7 @@ from folium.elements import JSCSSMixin from folium.features import FeatureGroup, GeoJson, TopoJson +from folium.folium import Map from folium.plugins import MarkerCluster from folium.template import Template from folium.utilities import remove_empty @@ -122,9 +123,17 @@ def __init__( self.placeholder = placeholder self.collapsed = collapsed self.options = remove_empty(**kwargs) - self.test_params() - def test_params(self): + def test_params(self, keys): + if keys is not None and self.search_label is not None: + assert self.search_label in keys, ( + f"The label '{self.search_label}' was not " f"available in {keys}" "" + ) + assert isinstance( + self._parent, Map + ), "Search can only be added to folium Map objects." + + def render(self, **kwargs): if isinstance(self.layer, GeoJson): keys = tuple(self.layer.data["features"][0]["properties"].keys()) elif isinstance(self.layer, TopoJson): @@ -136,8 +145,6 @@ def test_params(self): ) # noqa else: keys = None + self.test_params(keys=keys) - if keys is not None and self.search_label is not None: - assert self.search_label in keys, ( - f"The label '{self.search_label}' was not " f"available in {keys}" "" - ) + super().render(**kwargs) diff --git a/tests/test_folium.py b/tests/test_folium.py index 96b09a06f8..58ad28c978 100644 --- a/tests/test_folium.py +++ b/tests/test_folium.py @@ -98,8 +98,8 @@ def test_init(self): assert self.m.width == (900, "px") assert self.m.left == (0, "%") assert self.m.top == (0, "%") - assert self.m.no_touch is False - assert self.m.disable_3d is False + assert self.m.global_switches.no_touch is False + assert self.m.global_switches.disable_3d is False assert self.m.font_size == "1.5rem" assert self.m.to_dict() == { "name": "Map", @@ -465,29 +465,29 @@ def test_global_switches(self): out = m._parent.render() out_str = "".join(out.split()) assert '"preferCanvas":true' in out_str - assert not m.no_touch - assert not m.disable_3d + assert not m.global_switches.no_touch + assert not m.global_switches.disable_3d m = folium.Map(no_touch=True) out = m._parent.render() out_str = "".join(out.split()) assert '"preferCanvas":false' in out_str - assert m.no_touch - assert not m.disable_3d + assert m.global_switches.no_touch + assert not m.global_switches.disable_3d m = folium.Map(disable_3d=True) out = m._parent.render() out_str = "".join(out.split()) assert '"preferCanvas":false' in out_str - assert not m.no_touch - assert m.disable_3d + assert not m.global_switches.no_touch + assert m.global_switches.disable_3d m = folium.Map(prefer_canvas=True, no_touch=True, disable_3d=True) out = m._parent.render() out_str = "".join(out.split()) assert '"preferCanvas":true' in out_str - assert m.no_touch - assert m.disable_3d + assert m.global_switches.no_touch + assert m.global_switches.disable_3d def test_json_request(self): """Test requests for remote GeoJSON files.""" diff --git a/tests/test_map.py b/tests/test_map.py index 1e099f9b93..cc3728586a 100644 --- a/tests/test_map.py +++ b/tests/test_map.py @@ -109,8 +109,7 @@ def test_popup_unicode(): def test_popup_sticky(): m = Map() popup = Popup("Some text.", sticky=True).add_to(m) - script = popup._template.module.__dict__.get("script", None) - rendered = script(this=popup, kwargs={}) + rendered = popup._template.render(this=popup, kwargs={}) expected = """ var {popup_name} = L.popup({{ "maxWidth": "100%", @@ -132,8 +131,7 @@ def test_popup_sticky(): def test_popup_show(): m = Map() popup = Popup("Some text.", show=True).add_to(m) - script = popup._template.module.__dict__.get("script", None) - rendered = script(this=popup, kwargs={}) + rendered = popup._template.render(this=popup, kwargs={}) expected = """ var {popup_name} = L.popup({{ "maxWidth": "100%","autoClose": false, @@ -153,8 +151,7 @@ def test_popup_show(): def test_popup_backticks(): m = Map() popup = Popup("back`tick`tick").add_to(m) - script = popup._template.module.__dict__.get("script", None) - rendered = script(this=popup, kwargs={}) + rendered = popup._template.render(this=popup, kwargs={}) expected = """ var {popup_name} = L.popup({{ "maxWidth": "100%", @@ -173,8 +170,7 @@ def test_popup_backticks(): def test_popup_backticks_already_escaped(): m = Map() popup = Popup("back\\`tick").add_to(m) - script = popup._template.module.__dict__.get("script", None) - rendered = script(this=popup, kwargs={}) + rendered = popup._template.render(this=popup, kwargs={}) expected = """ var {popup_name} = L.popup({{ "maxWidth": "100%",