Skip to content

Commit 7238e82

Browse files
authored
fix(python): trim Layer.source redaction cost and document save_project (#1772)
* Address Claude review feedback - Sweep only the source field in Layer.source. redact_layer copied an inlined geojson blob first and then discarded it, which .data pays for deliberately but .source has no reason to. - Document that the top-level save_project writes verbatim, credentials included, and why: it is the lossless primitive the MCP server round-trips a user's own file through, where redacting on every edit would strip their own API keys. Point at Map.save_project and redact_credentials for the sharing case. - Sync docs/python.md's Map API table with the methods this PR adds; it had drifted from the expanded table in python/README.md. * Address CodeRabbit review feedback Fix the remove_layer row in python/README.md rather than docs/python.md. The two tables disagreed and the README was the wrong one: the parameter is `layer_id`, so `remove_layer(layer=...)` raises TypeError. Both now match the signature, and the description already says it accepts an id, name, or handle.
1 parent eeb9afd commit 7238e82

5 files changed

Lines changed: 35 additions & 4 deletions

File tree

docs/python.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,11 @@ m.on_layer_change(lambda e: print("layers", e["layerIds"]))
253253
| `add_colormap(colormap, vmin=, vmax=, label=, **kwargs)` | Add a colorbar from a named colormap (leafmap-style alias of `add_colorbar`). |
254254
| `set_center(lng, lat, zoom=None)` | Center (and optionally zoom) the map. |
255255
| `set_center_zoom(lng, lat, zoom=None)` | Alias of `set_center` (leafmap compatibility). |
256-
| `remove_layer(layer_id)` / `clear_layers()` | Remove layers. |
256+
| `set_zoom(zoom)` / `set_bearing(bearing)` / `set_pitch(pitch)` / `fit_project_bounds(bounds)` | Persist camera changes without requiring the widget to be displayed. |
257+
| `center` / `zoom` / `bearing` / `pitch` / `basemap` / `name` | Read persisted project and camera state; `name` is writable. |
258+
| `rename_layer(layer, name)` / `move_layer(layer, index)` / `duplicate_layer(layer, name=)` / `show_layer(layer)` / `hide_layer(layer)` | Manage layers by id, name, or `Layer` handle. |
259+
| `layer_properties(layer)` / `column_values(layer, column)` / `describe()` | Inspect inlined data and summarize a project without a browser round trip. |
260+
| `remove_layer(layer_id)` / `clear_layers()` | Remove one layer by id, name, or handle, or remove all layers. |
257261
| `to_project(keep_credentials=False)` | Return the current project as a dict, credentials redacted unless `keep_credentials=True`. |
258262
| `load_project(src)` | Replace the project from a dict, JSON string, or `.geolibre.json` path. |
259263
| `save_project(path, keep_credentials=False)` | Write the current project to a `.geolibre.json` file, credentials redacted unless `keep_credentials=True`. |

python/README.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ m.to_project()["mapView"]["center"]
8888
| `layer_names` / `find_layer(name)` / `set_layer_visibility` / `set_layer_opacity` | Inspect and update layers conveniently. |
8989
| `rename_layer` / `move_layer` / `duplicate_layer` / `show_layer` / `hide_layer` | Manage layers by id, name, or `Layer` handle. |
9090
| `layer_properties(layer)` / `column_values(layer, column)` / `describe()` | Inspect inlined data and summarize a project without a browser round trip. |
91-
| `remove_layer(layer)` / `clear_layers()` | Remove one layer by id, name, or handle, or remove all layers. |
91+
| `remove_layer(layer_id)` / `clear_layers()` | Remove one layer by id, name, or handle, or remove all layers. |
9292
| `center` / `zoom` / `bearing` / `pitch` / `basemap` / `name` | Read persisted project and camera state; `name` is writable. |
9393
| `set_zoom` / `set_bearing` / `set_pitch` / `fit_project_bounds` | Persist camera changes without requiring the widget to be displayed. |
9494
| `to_project()` / `load_project(src)` / `save_project(path)` | Project I/O. |
@@ -126,6 +126,12 @@ print(describe_project(project))
126126
save_project("copy.geolibre.json", project)
127127
```
128128

129+
These are the lossless file primitives: unlike `Map.save_project`, the top-level
130+
`save_project` writes the project **verbatim**, credentials included, so that
131+
editing a project in place cannot strip your own API keys out of it. Pass a
132+
project through `geolibre.project.redact_credentials` first if the file is going
133+
anywhere untrusted, or use `Map.save_project`, which redacts by default.
134+
129135
## Notes
130136

131137
- The bundled app is served from a localhost HTTP server, so the interactive

python/src/geolibre/authoring.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,19 @@ def save_project(path: str | Path, project: dict[str, Any]) -> Path:
115115
can approach ``MAX_PROJECT_BYTES``, and the MCP server rewrites the whole
116116
file on every edit, so a truncating write is a real way to lose work.
117117
118+
Note:
119+
This writes *verbatim*, credentials included. It is the lossless
120+
primitive the MCP server round-trips a user's own project file through,
121+
where stripping an API key on every small edit would quietly destroy the
122+
file's usefulness. :meth:`geolibre.Map.save_project` is the counterpart
123+
for producing a file to share: it redacts unless
124+
``keep_credentials=True``. Run a project through
125+
:func:`geolibre.project.redact_credentials` before calling this if the
126+
result is going anywhere untrusted.
127+
118128
Args:
119129
path: Destination path.
120-
project: The project dict to serialize.
130+
project: The project dict to serialize, written as given.
121131
122132
Returns:
123133
The resolved path written to.

python/src/geolibre/geolibre.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2525,7 +2525,9 @@ def source(self) -> Any:
25252525
into an output that often gets committed or shared. Read
25262526
:attr:`Map.project` for the record exactly as stored.
25272527
"""
2528-
return _project.redact_layer(self._layer()).get("source")
2528+
# Sweep the one field rather than the whole layer: `redact_layer` would
2529+
# copy an inlined geojson blob first, only to discard it here.
2530+
return _project.redact_layer_field(self._layer().get("source"))
25292531

25302532
@property
25312533
def data(self) -> dict[str, Any]:

python/src/geolibre/project.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,15 @@ def _sweep_layer_credentials(layer: dict[str, Any]) -> None:
182182
layer[field] = _redact_config(layer[field])
183183

184184

185+
def redact_layer_field(value: Any) -> Any:
186+
"""Return one of a layer's config fields, detached and swept.
187+
188+
The single-field counterpart to :func:`redact_layer`, for a read that wants
189+
only ``source`` and should not pay to copy an inlined GeoJSON blob first.
190+
"""
191+
return _redact_config(value)
192+
193+
185194
def redact_layer(layer: dict[str, Any]) -> dict[str, Any]:
186195
"""Return a detached copy of one layer, safe to display or hand to others.
187196

0 commit comments

Comments
 (0)