From e43d0f782f1c1d90b46aa48cf84d742148aef431 Mon Sep 17 00:00:00 2001 From: RAJVEER42 Date: Thu, 22 Oct 2026 20:00:31 +0200 Subject: [PATCH 1/3] grass.jupyter: Style vectors the same way on both backends Adds color, fill_color, width, opacity and fill_opacity to add_vector, using the d.vect names. Styling a vector meant writing backend specific code until now, a style_function for folium or a style dictionary for ipyleaflet. The translation to Leaflet names is a plain function with no backend imports, so it can be tested on its own. Each backend then takes the result in the form it wants. Replaces the shim which did this for opacity alone. A style_function or a style given directly still wins, so existing code keeps its behavior. This is the first step of #7626 and covers lines and areas. Point styling and colouring by a column come next. --- python/grass/jupyter/interactivemap.py | 50 ++++++++++++++--- .../tests/interactivemap_vector_style_test.py | 55 +++++++++++++++++++ 2 files changed, 97 insertions(+), 8 deletions(-) create mode 100644 python/grass/jupyter/tests/interactivemap_vector_style_test.py diff --git a/python/grass/jupyter/interactivemap.py b/python/grass/jupyter/interactivemap.py index 0813dc59646..55cb57222a7 100644 --- a/python/grass/jupyter/interactivemap.py +++ b/python/grass/jupyter/interactivemap.py @@ -142,6 +142,35 @@ def add_to(self, interactive_map): interactive_map.add(image) +# Leaflet path options for the styling d.vect exposes. The names on the left +# are the ones a GRASS user already knows from d.vect, the ones on the right +# are what both backends pass to Leaflet. +VECTOR_STYLE_OPTIONS = { + "color": "color", + "fill_color": "fillColor", + "width": "weight", + "opacity": "opacity", + "fill_opacity": "fillOpacity", +} + + +def vector_style(options): + """Translate GRASS vector style options to Leaflet path options + + Takes the recognized options out of *options* and returns them as a + dictionary Leaflet understands. Options which are not about style are + left in place, so what remains can go to the backend unchanged. + + :param dict options: keyword arguments given to a vector layer + :return dict: Leaflet path options, empty if none were given + """ + return { + leaflet_name: options.pop(name) + for name, leaflet_name in VECTOR_STYLE_OPTIONS.items() + if name in options + } + + class Vector(Layer): """Adds vectors to a folium or ipyleaflet map. @@ -172,9 +201,15 @@ def __init__( def add_to(self, interactive_map): """Add vector to map""" + # The two backends take styling differently, folium through a + # callable and ipyleaflet through a dictionary, so the same options + # are translated once here and handed to each in its own form. + style = vector_style(self._layer_kwargs) if get_backend(interactive_map) == "folium": import folium # pylint: disable=import-outside-toplevel + if style and "style_function" not in self._layer_kwargs: + self._layer_kwargs["style_function"] = lambda _feature: style folium.GeoJson( str(self._filename), name=self._title, **self._layer_kwargs ).add_to(interactive_map) @@ -183,14 +218,13 @@ def add_to(self, interactive_map): with open(self._filename, encoding="utf-8") as file: data = json.load(file) - # allow using opacity directly to keep interface - # consistent for both backends - if "opacity" in self._layer_kwargs: - opacity = self._layer_kwargs.pop("opacity") - if "style" in self._layer_kwargs: - self._layer_kwargs["style"]["opacity"] = opacity - else: - self._layer_kwargs["style"] = {"opacity": opacity} + if style: + # An explicitly given style wins, so a user who already + # passes one keeps the behavior they had. + self._layer_kwargs["style"] = { + **style, + **self._layer_kwargs.get("style", {}), + } geo_json = ipyleaflet.GeoJSON( data=data, name=self._title, **self._layer_kwargs ) diff --git a/python/grass/jupyter/tests/interactivemap_vector_style_test.py b/python/grass/jupyter/tests/interactivemap_vector_style_test.py new file mode 100644 index 00000000000..4a3dc497bc4 --- /dev/null +++ b/python/grass/jupyter/tests/interactivemap_vector_style_test.py @@ -0,0 +1,55 @@ +# SPDX-FileCopyrightText: 2026 Rajveer Bishnoi +# SPDX-License-Identifier: GPL-2.0-or-later + +"""Test the vector style options shared by the two interactive backends""" + +import pytest + +from grass.jupyter.interactivemap import vector_style + + +def test_translates_every_option(): + """Each option becomes the Leaflet name for it""" + options = { + "color": "black", + "fill_color": "#ffff00", + "width": 2, + "opacity": 0.8, + "fill_opacity": 0.4, + } + assert vector_style(options) == { + "color": "black", + "fillColor": "#ffff00", + "weight": 2, + "opacity": 0.8, + "fillOpacity": 0.4, + } + + +def test_takes_the_options_it_used(): + """The translated options are removed, the rest are left alone""" + options = {"color": "red", "name": "roads", "style_function": None} + vector_style(options) + assert options == {"name": "roads", "style_function": None} + + +def test_without_style_options(): + """Options which are not about style give an empty style""" + options = {"name": "roads"} + assert vector_style(options) == {} + assert options == {"name": "roads"} + + +@pytest.mark.parametrize( + ("option", "leaflet_name"), + [ + ("color", "color"), + ("fill_color", "fillColor"), + ("width", "weight"), + ("opacity", "opacity"), + ("fill_opacity", "fillOpacity"), + ], +) +def test_one_option_at_a_time(option, leaflet_name): + """One option on its own gives one Leaflet option""" + assert vector_style({option: 1}) == {leaflet_name: 1} From bf221b7e1e18422a28882c0a4dc69500dcabd725 Mon Sep 17 00:00:00 2001 From: RAJVEER42 Date: Mon, 26 Oct 2026 11:26:10 +0100 Subject: [PATCH 2/3] grass.jupyter: Keep the named style options winning over style The opacity shim set style["opacity"] from the opacity argument, so the argument won over the same key in a style passed directly. The merge introduced with the named options had it the other way around, which changed what opacity=0.4 with style={"opacity": 0.9} does on ipyleaflet. Swaps the merge order back and covers it with a test through add_to, along with one for keys that only the style sets. folium keeps a style_function given directly, since a callable is not something a dictionary can merge into. --- python/grass/jupyter/interactivemap.py | 8 +++-- .../tests/interactivemap_vector_style_test.py | 34 +++++++++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/python/grass/jupyter/interactivemap.py b/python/grass/jupyter/interactivemap.py index 55cb57222a7..c1a105212f4 100644 --- a/python/grass/jupyter/interactivemap.py +++ b/python/grass/jupyter/interactivemap.py @@ -219,11 +219,13 @@ def add_to(self, interactive_map): with open(self._filename, encoding="utf-8") as file: data = json.load(file) if style: - # An explicitly given style wins, so a user who already - # passes one keeps the behavior they had. + # The named options win over the same key in a style given + # directly, which is what the opacity shim did before them. + # folium cannot do the same, since a style_function is a + # callable there is no sensible way to merge into. self._layer_kwargs["style"] = { - **style, **self._layer_kwargs.get("style", {}), + **style, } geo_json = ipyleaflet.GeoJSON( data=data, name=self._title, **self._layer_kwargs diff --git a/python/grass/jupyter/tests/interactivemap_vector_style_test.py b/python/grass/jupyter/tests/interactivemap_vector_style_test.py index 4a3dc497bc4..5f2bf84b84b 100644 --- a/python/grass/jupyter/tests/interactivemap_vector_style_test.py +++ b/python/grass/jupyter/tests/interactivemap_vector_style_test.py @@ -5,6 +5,7 @@ import pytest +from grass.jupyter import interactivemap from grass.jupyter.interactivemap import vector_style @@ -53,3 +54,36 @@ def test_without_style_options(): def test_one_option_at_a_time(option, leaflet_name): """One option on its own gives one Leaflet option""" assert vector_style({option: 1}) == {leaflet_name: 1} + + +def test_named_option_wins_over_style(simple_dataset): + """A named option overrides the same key in a style given directly + + This is what the opacity shim did before the named options existed. + """ + ipyleaflet = pytest.importorskip("ipyleaflet") + + m = interactivemap.InteractiveMap(use_region=True, map_backend="ipyleaflet") + m.add_vector(simple_dataset.vector_name, opacity=0.4, style={"opacity": 0.9}) + + layer = next( + layer + for layer in reversed(m.map.layers) + if isinstance(layer, ipyleaflet.GeoJSON) + ) + assert layer.style["opacity"] == 0.4 + + +def test_style_keeps_keys_the_options_do_not_set(simple_dataset): + """Keys only present in a style given directly survive""" + ipyleaflet = pytest.importorskip("ipyleaflet") + + m = interactivemap.InteractiveMap(use_region=True, map_backend="ipyleaflet") + m.add_vector(simple_dataset.vector_name, opacity=0.4, style={"dashArray": "5"}) + + layer = next( + layer + for layer in reversed(m.map.layers) + if isinstance(layer, ipyleaflet.GeoJSON) + ) + assert layer.style == {"opacity": 0.4, "dashArray": "5"} From 413aec57814b408f2681c6a11de5ef6917ff6e7e Mon Sep 17 00:00:00 2001 From: RAJVEER42 Date: Tue, 27 Oct 2026 09:24:24 +0100 Subject: [PATCH 3/3] grass.jupyter: Treat a style given as None as the backend default Both branches asked whether the key was there rather than whether it held anything, so style=None raised a TypeError on ipyleaflet and style_function=None dropped the named options on folium. Both are how a caller asks for the backend default. Also applies the named options on top of a style_function instead of discarding them, which is what the ipyleaflet branch already did with style, so the same call now means the same thing on either backend. --- python/grass/jupyter/interactivemap.py | 24 ++++++--- .../tests/interactivemap_vector_style_test.py | 49 +++++++++++++++++++ 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/python/grass/jupyter/interactivemap.py b/python/grass/jupyter/interactivemap.py index c1a105212f4..a146f32e4f5 100644 --- a/python/grass/jupyter/interactivemap.py +++ b/python/grass/jupyter/interactivemap.py @@ -208,8 +208,19 @@ def add_to(self, interactive_map): if get_backend(interactive_map) == "folium": import folium # pylint: disable=import-outside-toplevel - if style and "style_function" not in self._layer_kwargs: - self._layer_kwargs["style_function"] = lambda _feature: style + if style: + # Both backends apply the named options on top of whatever the + # backend's own form produces, so the same call means the same + # thing on either one. A style_function is a callable, so it is + # wrapped rather than merged. + given = self._layer_kwargs.get("style_function") + if given is None: + self._layer_kwargs["style_function"] = lambda _feature: style + else: + self._layer_kwargs["style_function"] = lambda feature: { + **given(feature), + **style, + } folium.GeoJson( str(self._filename), name=self._title, **self._layer_kwargs ).add_to(interactive_map) @@ -219,12 +230,11 @@ def add_to(self, interactive_map): with open(self._filename, encoding="utf-8") as file: data = json.load(file) if style: - # The named options win over the same key in a style given - # directly, which is what the opacity shim did before them. - # folium cannot do the same, since a style_function is a - # callable there is no sensible way to merge into. + # `or {}` because passing style=None is how a caller asks for + # the backend default, and a missing key and an empty value + # have to mean the same thing here. self._layer_kwargs["style"] = { - **self._layer_kwargs.get("style", {}), + **(self._layer_kwargs.get("style") or {}), **style, } geo_json = ipyleaflet.GeoJSON( diff --git a/python/grass/jupyter/tests/interactivemap_vector_style_test.py b/python/grass/jupyter/tests/interactivemap_vector_style_test.py index 5f2bf84b84b..8659d6ea257 100644 --- a/python/grass/jupyter/tests/interactivemap_vector_style_test.py +++ b/python/grass/jupyter/tests/interactivemap_vector_style_test.py @@ -87,3 +87,52 @@ def test_style_keeps_keys_the_options_do_not_set(simple_dataset): if isinstance(layer, ipyleaflet.GeoJSON) ) assert layer.style == {"opacity": 0.4, "dashArray": "5"} + + +def test_style_none_is_the_backend_default(simple_dataset): + """style=None means the default, not a style to merge into""" + ipyleaflet = pytest.importorskip("ipyleaflet") + + m = interactivemap.InteractiveMap(use_region=True, map_backend="ipyleaflet") + m.add_vector(simple_dataset.vector_name, color="red", style=None) + + layer = next( + layer + for layer in reversed(m.map.layers) + if isinstance(layer, ipyleaflet.GeoJSON) + ) + assert layer.style == {"color": "red"} + + +def test_named_option_applies_over_a_style_function(simple_dataset): + """On folium the named options are applied on top of a style_function""" + pytest.importorskip("folium") + + m = interactivemap.InteractiveMap(use_region=True, map_backend="folium") + m.add_vector( + simple_dataset.vector_name, + color="red", + style_function=lambda _feature: {"dashArray": "6 4"}, + ) + + layer = next( + child + for child in m.map._children.values() + if child.__class__.__name__ == "GeoJson" + ) + assert layer.style_function(None) == {"dashArray": "6 4", "color": "red"} + + +def test_style_function_none_is_the_backend_default(simple_dataset): + """style_function=None means the default, so the named options still apply""" + pytest.importorskip("folium") + + m = interactivemap.InteractiveMap(use_region=True, map_backend="folium") + m.add_vector(simple_dataset.vector_name, color="red", style_function=None) + + layer = next( + child + for child in m.map._children.values() + if child.__class__.__name__ == "GeoJson" + ) + assert layer.style_function(None) == {"color": "red"}