Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/claude-autofix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,10 @@ jobs:
| select(.starter.nodes[0] | trusted)
| select(.comments.nodes[-1]
| (.author.__typename == "Bot" and (.body | startswith("**Claude autofix ("))) | not)
# A thread a maintainer left for the release quality pass (its last reply ends with
# the quality-sweep marker) waits for that pass, not for autofix.
| select(.comments.nodes[-1]
| (trusted and ((.body // "") | test("<!-- quality-sweep -->\\s*$"))) | not)
Comment thread
giswqs marked this conversation as resolved.
Resolved
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
| { id, path, line: (.line // .originalLine), outdated: .isOutdated,
comments: (.comments.totalCount as $n | .starter.nodes[0] as $s
| [ $s, (.comments.nodes[] | select(.id != $s.id)) ]
Expand Down
4 changes: 3 additions & 1 deletion apps/rustgis-python/python/rustgis/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,9 @@ def add_tiles(self, url: str, name: Optional[str] = None, **params: Any) -> Any:
**params: More ``map.addTiles`` parameters (``kind``, ``attribution``, ...).

Returns:
The tile layer's position, kind and name.
The tile layer's position, kind and name. An archive (PMTiles or MBTiles)
added without an ``id`` also reports ``zoomed``, whether its bounds framed
the map.
Comment thread
giswqs marked this conversation as resolved.
Resolved
"""
if name is not None:
params["name"] = name
Expand Down
4 changes: 2 additions & 2 deletions crates/analysis/src/control.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ pub fn install_progress(control: Option<Arc<Control>>) -> rustgis_data::progress
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct Cancelled;

/// What [`crate::run_with`] says of a run that was cancelled (it names the tool and how far it
/// came).
/// The error text the `From<Cancelled>` conversions carry. [`crate::run_with`] replaces it with
/// its own "… was cancelled" message, which names the tool and how far it came.
pub(crate) const CANCELLED: &str = "cancelled";

impl From<Cancelled> for String {
Expand Down
36 changes: 34 additions & 2 deletions crates/engine/src/cmd/workflows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ pub fn commands() -> Vec<C> {
.params(r#"{"name"?: model (default: the last), "graph"?: run this graph instead (as model.new)}"#),
C::new("model.validate", "Validate Model", "Analyze › Geoprocessing", |s, p| {
let (_, g, _) = model_graph(s, p)?;
note_passed_graph(s, p);
let problems = crate::model::validate(s, &g);
Ok(json!({"ok": problems.is_empty(), "problems": problems}))
}).roles(&[crate::ParamRole { key: "graph", role: crate::Role::ModelGraph }]).effects(crate::policy::Effects::NONE)
Expand Down Expand Up @@ -443,9 +444,38 @@ pub fn gallery_list(kind: &str) -> CmdResult {
Ok(Value::Array(out))
}

/// Tells the user what a graph passed to a command lost when it was read: one notice for the
/// nodes and one for the connections, each with a count.
fn notify_graph_dropped(s: &mut Session, dropped: crate::model::GraphDropped) {
if dropped.nodes > 0 {
s.notify(format!("model graph: {} node(s) left out: unreadable, without an id, or with the id of another", dropped.nodes), false);
}
if dropped.edges > 0 {
s.notify(
format!("model graph: {} connection(s) left out: unreadable, to a missing node or itself, or repeating another", dropped.edges),
Comment thread
giswqs marked this conversation as resolved.
Resolved
false,
);
}
}

/// Reads the `graph` a command was passed, if any, and says what reading it left out.
fn read_graph(s: &mut Session, p: &Value) -> Result<Option<crate::project::ModelGraph>, String> {
let Some(g) = p.get("graph").filter(|g| !g.is_null()) else { return Ok(None) };
let (graph, dropped) = crate::model::graph_from_json_counted(g)?;
notify_graph_dropped(s, dropped);
Ok(Some(graph))
}

/// Says what the graph a command was passed (rather than a saved model's) lost when read.
fn note_passed_graph(s: &mut Session, p: &Value) {
if let Some(Ok((_, dropped))) = p.get("graph").filter(|g| !g.is_null()).map(crate::model::graph_from_json_counted) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit (performance / clarity): every command that takes a graph now parses it twice. read_graph (or model_graph) reads it, and note_passed_graph runs graph_from_json_counted again just to get the counts, including the layout pass. It is harmless at the 2000-node cap, but model_graph could return the GraphDropped it already computes, which would remove the second parse in model.run, model.validate and model.export.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left for the release quality pass: the second parse is bounded by the node and connection caps, and returning the counts from model_graph is a refactor.

notify_graph_dropped(s, dropped);
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

fn new_model(s: &mut Session, p: &Value) -> CmdResult {
let base = ps(p, "name").map(str::trim).filter(|n| !n.is_empty()).unwrap_or("Model").chars().take(200).collect::<String>();
let graph = p.get("graph").filter(|g| !g.is_null()).map(crate::model::graph_from_json).transpose()?;
let graph = read_graph(s, p)?;
let steps: Vec<ModelStep> = match p.get("steps") {
_ if graph.is_some() => graph.as_ref().map(crate::model::graph_to_steps).unwrap_or_default(),
Some(v) => serde_json::from_value(v.clone()).map_err(|e| format!("steps: {e}"))?,
Expand Down Expand Up @@ -636,6 +666,7 @@ fn run_model_here(s: &mut Session, p: &Value) -> CmdResult {
}
}
let (name, _, trusted) = model_graph(s, p)?;
note_passed_graph(s, p);
let op = s.authorize_model(p)?;
Comment thread
giswqs marked this conversation as resolved.
Resolved
if trusted || p.get("graph").is_some_and(|g| !g.is_null()) {
crate::model::run(s, op)
Expand All @@ -654,7 +685,7 @@ pub(crate) fn model_save_names(p: &Value) -> Result<(String, String), String> {
/// Saves a graph as a model, replacing the one of that name (or adding it).
fn save_model(s: &mut Session, p: &Value) -> CmdResult {
let (name, new_name) = model_save_names(p)?;
let g = crate::model::graph_from_json(p.get("graph").ok_or("graph is required")?)?;
let g = read_graph(s, p)?.ok_or("graph is required")?;
Comment thread
giswqs marked this conversation as resolved.
Resolved
let i = s.project.models.iter().position(|m| m.name.eq_ignore_ascii_case(&name));
if !new_name.eq_ignore_ascii_case(&name) && s.project.models.iter().any(|m| m.name.eq_ignore_ascii_case(&new_name)) {
return Err(format!("a model named '{new_name}' already exists"));
Expand All @@ -673,6 +704,7 @@ fn save_model(s: &mut Session, p: &Value) -> CmdResult {
/// A model as a `*.model.json` file: written to `path`, or its text returned.
fn export_model(s: &mut Session, p: &Value) -> CmdResult {
let (name, g, _) = model_graph(s, p)?;
note_passed_graph(s, p);
let (text, report) =
crate::durable::output_owned(g, |g| serde_json::to_string_pretty(&crate::model::to_file(&name, g)).map_err(|e| e.to_string()))?;
crate::durable::notify(s, &report);
Comment thread
giswqs marked this conversation as resolved.
Resolved
Expand Down
31 changes: 28 additions & 3 deletions crates/engine/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,21 @@ pub fn graph_to_steps(g: &ModelGraph) -> Vec<ModelStep> {
/// edges to missing nodes or to themselves. Bounded in size.
/// Repeated connections into many-valued input ports retain their multiplicity.
pub fn graph_from_json(v: &Value) -> Result<ModelGraph, String> {
graph_from_json_counted(v).map(|(g, _)| g)
}

/// What [`graph_from_json_counted`] left out of a graph.
#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
pub struct GraphDropped {
/// Nodes that could not be read, had no id, or repeated another's id.
pub nodes: usize,
/// Connections that could not be read, joined a missing node or itself, or repeated another.
pub edges: usize,
}

/// Reads a graph as [`graph_from_json`] does, and counts the nodes and connections it left out,
/// so a command can say so. Errors when `nodes` is missing or either list is too long.
pub fn graph_from_json_counted(v: &Value) -> Result<(ModelGraph, GraphDropped), String> {
let nodes = v.get("nodes").and_then(Value::as_array).ok_or("a model graph needs \"nodes\"")?;
if nodes.len() > MAX_NODES {
return Err(format!("a model can have at most {MAX_NODES} nodes"));
Expand All @@ -486,10 +501,15 @@ pub fn graph_from_json(v: &Value) -> Result<ModelGraph, String> {
return Err(format!("a model can have at most {MAX_EDGES} connections"));
}
let mut g = ModelGraph::default();
let mut dropped = GraphDropped::default();
Comment thread
giswqs marked this conversation as resolved.
Resolved
for n in nodes {
let Ok(mut node) = serde_json::from_value::<ModelNode>(n.clone()) else { continue };
let Ok(mut node) = serde_json::from_value::<ModelNode>(n.clone()) else {
dropped.nodes += 1;
continue;
};
node.id = node.id.trim().chars().take(MAX_NODE_ID).collect();
Comment thread
giswqs marked this conversation as resolved.
Resolved
if node.id.is_empty() || g.nodes.iter().any(|m| m.id == node.id) {
dropped.nodes += 1;
continue;
}
if !node.x.is_finite() || !node.y.is_finite() {
Expand All @@ -504,19 +524,24 @@ pub fn graph_from_json(v: &Value) -> Result<ModelGraph, String> {
g.nodes.push(node);
}
for e in edges {
let Ok(e) = serde_json::from_value::<ModelEdge>(e.clone()) else { continue };
let Ok(e) = serde_json::from_value::<ModelEdge>(e.clone()) else {
dropped.edges += 1;
continue;
};
let has = |id: &str| g.nodes.iter().any(|n| n.id == id);
let repeated_input =
g.nodes.iter().find(|n| n.id == e.to).is_some_and(|n| input_ports(n).iter().any(|p| p.id == e.to_port && p.many));
if e.from != e.to && has(&e.from) && has(&e.to) && !e.to_port.is_empty() && (repeated_input || !g.edges.contains(&e)) {
g.edges.push(e);
} else {
dropped.edges += 1;
}
}
// A graph with no positions (written by hand or by an agent) is laid out.
if g.nodes.iter().all(|n| n.x == 0.0 && n.y == 0.0) {
layout(&mut g);
}
Ok(g)
Ok((g, dropped))
}

/// How one node fared in a run.
Expand Down
47 changes: 47 additions & 0 deletions crates/engine/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25721,6 +25721,53 @@ fn dem_tiles_mosaic_clip_by_polygon_share_a_stretch_and_report_overviews() {
assert!(e.to_string().contains("overview"), "{e}");
}

/// A graph passed to a model command says what reading it left out (one notice per kind, with a
/// count); a clean graph says nothing.
#[test]
fn model_graph_drops_are_noticed() {
let mut s = world();
let clean = json!({
"nodes": [
{"id": "i1", "kind": "input", "x": 0, "y": 0, "layer": "Cities"},
{"id": "o1", "kind": "output", "x": 240, "y": 0, "name": "Kept"}
],
"edges": [{"from": "i1", "fromPort": "out", "to": "o1", "toPort": "in"}]
});
let said = |s: &Session, mark: u64, what: &str| s.notices_since(mark).iter().filter(|n| n.text.contains(what)).count();
let mark = s.notice_mark();
s.run("model.new", &json!({"name": "Clean", "graph": clean})).unwrap();
s.run("model.save", &json!({"name": "Clean", "graph": clean})).unwrap();
s.run("model.validate", &json!({"graph": clean})).unwrap();
assert_eq!(said(&s, mark, "left out"), 0, "{:?}", s.notices_since(mark));

let lossy = json!({
"nodes": [
{"id": "i1", "kind": "input", "x": 0, "y": 0, "layer": "Cities"},
{"id": "i1", "kind": "input", "x": 10, "y": 0, "layer": "Countries"},
{"id": " ", "kind": "input", "x": 20, "y": 0, "layer": "Cities"},
{"kind": 7},
{"id": "o1", "kind": "output", "x": 240, "y": 0, "name": "Kept"}
],
"edges": [
{"from": "i1", "fromPort": "out", "to": "o1", "toPort": "in"},
{"from": "i1", "fromPort": "out", "to": "missing", "toPort": "in"},
{"from": "o1", "fromPort": "out", "to": "o1", "toPort": "in"},
"not an edge"
]
});
for (cmd, params) in [
("model.new", json!({"name": "Lossy", "graph": lossy})),
("model.save", json!({"name": "Lossy", "graph": lossy})),
("model.validate", json!({"graph": lossy})),
("model.export", json!({"graph": lossy})),
] {
let mark = s.notice_mark();
s.run(cmd, &params).unwrap();
assert_eq!(said(&s, mark, "3 node(s) left out"), 1, "{cmd}: {:?}", s.notices_since(mark));
assert_eq!(said(&s, mark, "3 connection(s) left out"), 1, "{cmd}: {:?}", s.notices_since(mark));
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
}

/// ModelBuilder graphs: validated, run in flow order, intermediate results dropped, outputs named.
fn model_graphs(s: &mut Session) {
let graph = json!({
Expand Down
11 changes: 10 additions & 1 deletion crates/geom/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,13 @@ pub fn format_area(m2: f64) -> String {
if !m2.is_finite() {
return "—".to_string();
}
if m2.abs().round() < 1.0e6 { format!("{m2:.0} m²") } else { format!("{:.1} km²", m2 / 1.0e6) }
if m2.abs().round() < 1.0e6 {
// A tiny negative area rounds to `-0`; it reads `0 m²`.
let whole = format!("{m2:.0}");
format!("{} m²", if whole == "-0" { "0" } else { &whole })
} else {
format!("{:.1} km²", m2 / 1.0e6)
}
}

/// Formats a lon/lat pair in decimal degrees with hemispheres: `12.345° E 45.678° N`.
Expand Down Expand Up @@ -1234,6 +1240,9 @@ mod tests {
assert_eq!(format_distance(-12_500.0), "-12.5 km");
assert_eq!(format_distance(-99_949.0), "-99.9 km");
assert_eq!(format_distance(-99_960.0), "-100 km");
assert_eq!(format_area(-0.3), "0 m²");
assert_eq!(format_area(-0.0), "0 m²");
assert_eq!(format_area(-0.6), "-1 m²");
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
assert_eq!(format_area(-999_999.4), "-999999 m²");
assert_eq!(format_area(-999_999.6), "-1.0 km²");
assert_eq!(format_area(-1.0e6), "-1.0 km²");
Expand Down
2 changes: 1 addition & 1 deletion crates/tiles/src/style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1150,7 +1150,7 @@ fn parse_sprites(entries: &[Json]) -> (Vec<(String, String)>, Vec<String>) {
}
if past > 0 {
left_out.push(format!(
"{past} of {total} sprites left out, so their icons and patterns aren't drawn: a style's first {MAX_SPRITES} are read"
"{past} of {total} sprites left out, so their icons and patterns aren't drawn: the style's first {MAX_SPRITES} that can be read are used"
Comment thread
giswqs marked this conversation as resolved.
Resolved
));
}
(sprites, left_out)
Expand Down
3 changes: 2 additions & 1 deletion docs/i18n.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ file-state Open Offen
```

Use `tl!("Open", context = "file-state")` at the call site. `tl_raw!` accepts the same context
for fragments, and `i18n::fmt_context("file-state", "{n} open", &[("n", count)])` formats a
for fragments, and
`i18n::fmt_context("file-state", "{n} open", &[("n", &count.to_string())])` formats a
contextual message. Contexts describe the sense of a word, not a translated label. When a
contextual entry is missing, lookup falls back to the ordinary translation, then to English.
Plugin catalogs keep their existing format and provide the ordinary fallback.
Expand Down
7 changes: 4 additions & 3 deletions docs/themes.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,9 @@ once a second. Without that folder, the app does not wake just to check for them
the folder while the app is idle, interact with the app or use **Reload Themes** to discover it. A save
that changes that metadata is picked up automatically. If a save preserves both size and
modification time, use **Reload Themes** to reread it. A theme in use that stops loading leaves the window on the nearest built-in
until it loads again: the button names the theme drawn, the menu gives the reason, and the choice
returns with the file.
until it loads again: the button names the theme drawn, and the choice returns with the file. A
file that is refused is listed in the menu with its reason; one deleted or moved away is not
listed, and only the fallback shows it is gone.

## A short theme

Expand Down Expand Up @@ -145,7 +146,7 @@ map's light or dark defaults. It is not inferred from the colours.
`metrics` are in points: item spacing, button padding, row height, corner radius and text sizes.
Corner radius is held between 0 and 32 and rounded to the nearest whole point; an adjustment is counted.
Sizes are held to what a window can draw, and a notice says when a value was held. A file over 64 KB
is not read.
is refused.

## Tokens

Expand Down
9 changes: 5 additions & 4 deletions docs/user-guide/analysis.md
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ gp.batch={"tool":"buffer","inputs":["Cities","Rivers"],"outputPattern":"{input}_
with a notice). In the browser, add a file as a layer first and use its layer name.
- `"folder"`: on desktop and the command line, a folder whose `.shp`, `.geojson`, `.json`,
`.gpkg`, `.fgb`, `.parquet`, `.geoparquet`, `.kml`, `.kmz`, `.gpx` and `.csv` files are run over
in alphabetical order. Subfolders and symbolic links are skipped, with a count.
in file-name order (byte order, so capitals come first). Subfolders and symbolic links are skipped, with a count.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
- `"outputPattern"` (or `"output_pattern"`): the name of each output layer, `"{input}_{tool}"` by
default, where `{input}` is the layer's name or the file's name without its extension and
`{tool}` the tool's id. It must include `{input}`. A name already taken gets a number (`_2`).
Expand Down Expand Up @@ -961,7 +961,8 @@ such as `exterior ring of part 1 crosses itself; 3 repeated vertices in ring 2 o
feature with more than 20 problems lists the first 20 and counts the rest. If the input already
has a `problem` or `detail` field, the new one gets the next free name (`problem_2`) and the
tool says so. When nothing is wrong, the output layer is empty and the message says how many
features were checked.
features were checked. `repeated_vertex` is not one of the OGC rules, which allow a point
repeated in a row: it is a data-quality check reported beside them.

| Code | What is wrong |
| --- | --- |
Expand Down Expand Up @@ -1190,7 +1191,7 @@ the environment does not set their output grids.
computer's memory, or a container's limit when it is lower: each plans for a quarter of it, never
below the web app's fixed limits and never above a ceiling.

| Limit | Fixed (web app, smallest) | Desktop: a quarter of memory at | Ceiling |
| Limit | Floor (web app value in brackets) | Desktop: a quarter of memory at | Ceiling |
|---|---|---|---|
| `rasterSamples`: samples a raster read holds (cells times bands) | 120 million (32 million in the web app) | 8 bytes a sample | 1 billion |
| `outputPixels`: pixels of a raster a tool makes | 64 million | 16 bytes a pixel | 512 million |
Expand Down Expand Up @@ -1410,7 +1411,7 @@ The Analyze tab's imagery groups open their tools in the Workflows pane:
- **Resampling:** `nearest` (the default: it never invents a value, which is right for classes),
`bilinear` or `cubic` sample each cell at its centre. A cell whose centre falls on a no-data
pixel (or on NaN, with or without a no-data value; in a colour image, no-data in every
channel) is no-data; otherwise they interpolate from the pixels around it that have a value,
colour channel, alpha aside) is no-data; otherwise they interpolate from the pixels around it that have a value,
as gdalwarp does. Going coarser, they skip the detail between cell centres and can give a jagged
result, where gdalwarp widens its kernel to smooth it. `average` and `mode`
take every pixel whose centre falls in the cell (no-data skipped), its mean or its most frequent
Expand Down
Loading
Loading