mirror of
https://github.com/storytold/lightcraft.git
synced 2026-10-28 20:17:18 -06:00
#2242: reverseGeocode maxDistance, and the 15 000-people limit is documented
GeoNames cities15000 holds towns of 15 000 people or more, so a photo in a smaller town got the nearest larger one in City (Portofino → Rapallo, 5.3 km), another municipality. metadata.reverseGeocode takes maxDistance (metres): past it, City and Sublocation stay empty, State and Country still fill, and the reply names the town as nearestCity. Hostile values are refused. The command help, the Fill Location tooltip and the parity row say what the list holds. Default unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AbegtYaPuwqZ81pGfELsFK
This commit is contained in:
@@ -115,6 +115,12 @@ fn fill(meta: &mut lightcraft_catalog::Meta, place: &lightcraft_geocode::Place,
|
||||
fn reverse(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
const C: &str = "metadata.reverseGeocode";
|
||||
let overwrite = bool_or(p, "overwrite", false);
|
||||
// the place list holds towns of 15 000 people or more: a photo in a smaller one finds the nearest
|
||||
// larger town, which may be another municipality; past this many metres City is left empty (#2242)
|
||||
let max_distance = match p.get("maxDistance") {
|
||||
None | Some(Value::Null) => None,
|
||||
Some(v) => Some(v.as_f64().filter(|m| m.is_finite() && *m > 0.0).ok_or_else(|| bad(C, "`maxDistance` must be a positive number of metres"))?),
|
||||
};
|
||||
let ids = s.targets(p)?;
|
||||
let todo: Vec<(PhotoId, LatLon)> = ids
|
||||
.iter()
|
||||
@@ -140,14 +146,24 @@ fn reverse(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
}
|
||||
t.progress(n as u64, todo.len() as u64);
|
||||
}
|
||||
let Some(place) = names.reverse(*at) else { continue };
|
||||
let Some(mut place) = names.reverse(*at) else { continue };
|
||||
// too far from the town found: State and Country still hold, City (and below) doesn't
|
||||
let too_far = max_distance.is_some_and(|m| place.distance > m);
|
||||
let nearest_city = too_far.then(|| std::mem::take(&mut place.city));
|
||||
if too_far {
|
||||
place.sublocation.clear();
|
||||
}
|
||||
let Some(ph) = s.catalog.photo(*id) else { continue };
|
||||
let mut meta = ph.meta.clone();
|
||||
fill(&mut meta, &place, overwrite);
|
||||
if meta != ph.meta {
|
||||
ops.push(Op::SetMeta { id: *id, meta: Box::new(meta) });
|
||||
}
|
||||
found.push(json!({"id": id.0, "place": place}));
|
||||
let mut entry = json!({"id": id.0, "place": place});
|
||||
if let Some(city) = nearest_city {
|
||||
entry["nearestCity"] = json!(city);
|
||||
}
|
||||
found.push(entry);
|
||||
}
|
||||
drop(task);
|
||||
let changed = ops.len();
|
||||
@@ -271,7 +287,7 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
"Fill Location from GPS",
|
||||
["Photo"],
|
||||
None,
|
||||
"{ids?, overwrite?: bool (also replace place fields that already have text; default false: only empty ones are filled), dryRun?, dataset?: folder} → {looked, found, changed, photos: [{id, place: {sublocation, city, state, country, iso, distance}}], attribution} — fills City / State / Country (and Sublocation when known) from GPS with the offline GeoNames place names (download them first); one undo step",
|
||||
"{ids?, overwrite?: bool (also replace place fields that already have text; default false: only empty ones are filled), dryRun?, dataset?: folder, maxDistance?: metres (past this distance from the town found, City is left empty and the town is given as nearestCity)} → {looked, found, changed, photos: [{id, place: {sublocation, city, state, country, iso, distance}, nearestCity?}], attribution} — fills City / State / Country (and Sublocation when known) from GPS with the offline GeoNames place names (download them first); one undo step. The place names list towns of 15 000 people or more (GeoNames cities15000): a photo taken in a smaller town gets the nearest larger one, up to 50 km away (`distance` says how far); `maxDistance` keeps such a town out of City",
|
||||
has_selection,
|
||||
reverse
|
||||
),
|
||||
@@ -343,6 +359,36 @@ mod tests {
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
/// Issue #2242: the place names hold only towns of 15 000 people or more, so a photo in a
|
||||
/// village finds the nearest larger town. `maxDistance` leaves City empty past that distance
|
||||
/// (State and Country still fill) and says which town it was.
|
||||
#[test]
|
||||
fn max_distance_leaves_a_far_town_out_of_city() {
|
||||
let mut s = Session::with_demo();
|
||||
let ids = s.visible_cloned();
|
||||
let dir = dataset("maxdist");
|
||||
// about 5 km north of Paris (like Portofino and Rapallo in the issue), and right in Paris
|
||||
set_gps(&mut s, ids[0], Some((48.90, 2.35)), "");
|
||||
set_gps(&mut s, ids[1], Some((48.855, 2.35)), "");
|
||||
let r = s.execute("metadata.reverseGeocode", &json!({"ids": [ids[0].0, ids[1].0], "dataset": dir, "maxDistance": 2000})).unwrap();
|
||||
let m0 = s.catalog.photo(ids[0]).unwrap().meta.clone();
|
||||
assert_eq!((m0.city.as_str(), m0.state.as_str(), m0.country.as_str()), ("", "Île-de-France", "France"));
|
||||
assert_eq!(s.catalog.photo(ids[1]).unwrap().meta.city, "Paris", "close enough");
|
||||
let far = r["photos"].as_array().unwrap().iter().find(|p| p["id"] == json!(ids[0].0)).unwrap();
|
||||
assert_eq!((far["place"]["city"].as_str(), far["nearestCity"].as_str()), (Some(""), Some("Paris")), "{far}");
|
||||
assert!(far["place"]["distance"].as_f64().unwrap() > 2000.0);
|
||||
// without it, the nearest town fills City as before
|
||||
s.undo_step().unwrap();
|
||||
s.execute("metadata.reverseGeocode", &json!({"ids": [ids[0].0], "dataset": dir})).unwrap();
|
||||
assert_eq!(s.catalog.photo(ids[0]).unwrap().meta.city, "Paris");
|
||||
// hostile values are refused, not taken as "no limit"
|
||||
for bad in [json!(-1), json!(0), json!("far"), json!(true)] {
|
||||
let e = s.execute("metadata.reverseGeocode", &json!({"ids": [ids[0].0], "dataset": dir, "maxDistance": bad})).unwrap_err();
|
||||
assert!(e.to_string().contains("maxDistance"), "{e}");
|
||||
}
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn explains_missing_data_and_gps() {
|
||||
let mut s = Session::with_demo();
|
||||
|
||||
@@ -771,7 +771,11 @@ fn info(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) {
|
||||
}
|
||||
});
|
||||
if text_button(ui, "fillLocation", crate::i18n::tr("Fill Location"), false)
|
||||
.on_hover_text(crate::i18n::tr("Fill empty City, State and Country from GPS (offline GeoNames place names)"))
|
||||
.on_hover_text(format!(
|
||||
"{}\n{}",
|
||||
crate::i18n::tr("Fill empty City, State and Country from GPS (offline GeoNames place names)"),
|
||||
crate::i18n::tr("The place names list towns of 15,000 people or more: in a smaller one, City gets the nearest larger town.")
|
||||
))
|
||||
.clicked()
|
||||
&& let Err(e) = app.run("metadata.reverseGeocode", json!({}))
|
||||
{
|
||||
|
||||
@@ -737,7 +737,7 @@ From `08-lightroom-classic-extras.md` (Classic-only features) and part 2 of `06-
|
||||
| LRC-MAP-LOCATIONS | Saved locations | P2 | ⬜ | | |
|
||||
| LRC-MAP-TRACKLOG | GPS track logs | P2 | ✅ | `cmd:photo.autoTagTracklog`, `cmd:photo.tagFromTracklog`, `crates/meta/src/gpx.rs` | Photo ▸ Auto-Tag from Tracklog…: a GPX 1.0 / 1.1 track log sets the GPS of the selected photos by capture time — interpolated between the points of a track segment, else the nearest point within `maxGap` (10 min); never across segment breaks. The camera's time zone comes from the photo (Exif offset) or is asked for; photos that already have a location keep it unless `replace`; `dryRun` previews; one undo step. No track drawn on a map (no Map module) |
|
||||
| LRC-MAP-FILTER | Location filter bar | P2 | ⬜ | | |
|
||||
| LRC-MAP-REVGEO | Reverse geocoding | P2 | 🟡 | `cmd:metadata.reverseGeocode` | offline from GeoNames (City / State / Country, no sublocation); no online provider |
|
||||
| LRC-MAP-REVGEO | Reverse geocoding | P2 | 🟡 | `cmd:metadata.reverseGeocode` | offline from GeoNames (City / State / Country, no sublocation); the list holds towns of 15 000 people or more, so a smaller town gets the nearest larger one (the tooltip and command help say so); `maxDistance` leaves City empty past a distance and names the town as `nearestCity` (issue #2242); no online provider |
|
||||
| LRC-BOOK-SETTINGS | Book settings | P2 | ⬜ | | |
|
||||
| LRC-BOOK-AUTOLAYOUT | Book auto layout | P2 | ⬜ | | |
|
||||
| LRC-BOOK-PAGE | Book pages & templates | P2 | ⬜ | | |
|
||||
|
||||
@@ -1,5 +1,13 @@
|
||||
# What's new in LightCraft
|
||||
|
||||
## 2026-10-18
|
||||
|
||||
### Fill Location from GPS: no more neighbouring town in City
|
||||
- The offline place names list towns of 15,000 people or more, so a photo taken in a smaller one got the nearest larger
|
||||
town in City, sometimes another municipality (issue #2242). The Fill Location tooltip and the command help now say so,
|
||||
and `metadata.reverseGeocode` takes `maxDistance` (metres): past it, City stays empty (State and Country still fill)
|
||||
and the reply names the town it found as `nearestCity`.
|
||||
|
||||
## 2026-10-14
|
||||
|
||||
### Model downloads time out on a name that doesn't resolve
|
||||
|
||||
Reference in New Issue
Block a user