From 3561bf5e0ee466f1addd0a2b484e05f8f13ade4a Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Wed, 19 Aug 2026 17:37:56 +0200 Subject: [PATCH 1/4] feat(metadata): capture date and place, with a map picker The Metadata panel authored the gear and the process but not when or where the frame was shot, and the export inherited the scan file's DateTimeOriginal, so a 1998 negative sorted by its 2026 scan date. A Capture card adds both. The date is a truncated ISO-8601 string, because a film date is usually partial: 1998, 1998-07, a full date, or a date and time with an optional offset. EXIF pads the parts the user left out, XMP photoshop:DateCreated keeps the truncated form, and negpy:CaptureDatePrecision says which it was. The scan's own timestamp moves to DateTimeDigitized, which is what it recorded. The place is a WGS-84 position plus city, state and country, picked in a map dialog: a hand-rolled OpenStreetMap tile widget (paintEvent plus urllib, no new dependency and no browser in the bundle), Nominatim search and reverse lookup, and a field that also takes a pasted coordinate pair or map link. Everything works with no network except the map itself. Coordinates go to the EXIF GPS IFD and XMP exif:GPS*, the names to photoshop:City/State/Country. Setting a place now clears the source GPS block whole, so a picked position cannot ship with the scan's altitude or heading beside it. The source position is read back too: it shows in the preview as Scan place and frames the picker's opening view, but is never adopted as the capture place, because where a frame was digitized is not where it was shot. Both facts are searchable as shot: and place:. shot: is ordered by the existing prefix comparison, so shot:>=1998-07 needs no date parsing, and a year-only date deliberately does not match a month bound. Dropping GPS from the TIFF extratags loop fixes a latent bug: a source GPS IFD was written as top-level TIFF tags 1-4, which are not TIFF tags. TIFF now carries the location in XMP, which is what DAMs read. Closes #899 --- docs/TEMPLATING.md | 2 + docs/USER_GUIDE.md | 9 +- negpy/desktop/settings_catalog.py | 12 + negpy/desktop/view/sidebar/files.py | 4 +- negpy/desktop/view/sidebar/metadata.py | 138 ++++++++- .../view/widgets/location_picker_dialog.py | 247 +++++++++++++++++ negpy/desktop/view/widgets/slippy_map.py | 261 ++++++++++++++++++ negpy/features/metadata/capture.py | 205 ++++++++++++++ negpy/features/metadata/exif_read.py | 36 +++ negpy/features/metadata/models.py | 10 + negpy/features/metadata/payload.py | 54 ++++ negpy/features/metadata/writer.py | 30 +- negpy/features/metadata/xmp.py | 23 +- negpy/services/assets/search.py | 7 +- negpy/services/export/templating.py | 5 + negpy/services/maps.py | 126 +++++++++ tests/metadata/test_capture.py | 151 ++++++++++ tests/metadata/test_writer.py | 87 ++++++ tests/test_asset_search.py | 55 ++++ tests/test_config_deserialization.py | 20 ++ tests/test_location_picker_dialog.py | 108 ++++++++ tests/test_maps.py | 114 ++++++++ tests/test_metadata_sidebar.py | 172 ++++++++++++ tests/test_slippy_map.py | 157 +++++++++++ tests/test_templating.py | 11 + 25 files changed, 2030 insertions(+), 14 deletions(-) create mode 100644 negpy/desktop/view/widgets/location_picker_dialog.py create mode 100644 negpy/desktop/view/widgets/slippy_map.py create mode 100644 negpy/features/metadata/capture.py create mode 100644 negpy/services/maps.py create mode 100644 tests/metadata/test_capture.py create mode 100644 tests/test_location_picker_dialog.py create mode 100644 tests/test_maps.py create mode 100644 tests/test_metadata_sidebar.py create mode 100644 tests/test_slippy_map.py diff --git a/docs/TEMPLATING.md b/docs/TEMPLATING.md index d8d1529a..1f6f3e2a 100644 --- a/docs/TEMPLATING.md +++ b/docs/TEMPLATING.md @@ -36,6 +36,8 @@ NegPy uses **Jinja2** for dynamic file naming in both the **Export** and **Scan* | `{{ push_pull }}` | Push/pull as an integer (−3…+3, 0 = Normal). | `1` | | `{{ scanning }}` | Scanning method note. | `DSLR copy-stand` | | `{{ exposure }}` | Exposure override text from Metadata. | `1/125s f/2.8` | +| `{{ capture_date }}` | Original capture date in YYYYMMDD. A partial date pads to the first day. Empty if unset. | `19980714` | +| `{{ capture_year }}` | Original capture year. Empty if unset. | `1998` | Gear and process values come from the **Metadata** panel, or from each file's saved metadata in a batch. An empty field renders as an empty string, so the separators around it collapse. NegPy strips path-unsafe characters from metadata values. diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 5d3f5841..af8fca38 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -128,6 +128,8 @@ Type a plain word and it matches the filename. Beyond that the box takes `field: | `camera:"Nikon F3"` | quote anything with a space | | `iso:>=400` | numeric fields also take `>`, `>=`, `<`, `<=` (`iso`, `frame`, `push`) | | `date:2024-03` · `date:>=2024` | by file date; a partial date is a prefix | +| `shot:1998` · `shot:>=1998-07` | by capture date from the Metadata panel, not the file date | +| `place:tokyo` | by capture city, state or country | | `roll:` `developer:` `lens:` `format:` `scanning:` | the rest of the Metadata panel | | `name:` `path:` `ext:tif` | file identity | | `keeper:` `rejected:` `edited:` | frames carrying that mark, or with a saved edit | @@ -819,6 +821,11 @@ Archival metadata for the **original analog capture** (camera, lens, film, proce * **Camera / Lens / Film stock**: pick from your library. Empty means not set. * **Manage…**: edit cameras, lenses, film stocks and presets. Starter data seeds into `~/NegPy/gear/` on first launch. +**Capture:** + +* **Date**: when the frame was shot — give only what you know: `1998`, `1998-07`, `1998-07-14` or `1998-07-14 16:30`, with an optional offset such as `+02:00`. An impossible date turns the field red and is not saved. EXIF `DateTimeOriginal` pads the missing parts; XMP `photoshop:DateCreated` keeps the truncated form and `negpy:CaptureDatePrecision` names it. The scan file's own timestamp moves to `DateTimeDigitized`. +* **Place**: the capture location. **Map…** opens a map to search a place name, click a position or paste coordinates, **Clear** empties it, and the field itself accepts a pasted coordinate pair or an OpenStreetMap/Google Maps link. Coordinates are written to the EXIF GPS tags and XMP `exif:GPS*`, the names to XMP `photoshop:City`/`State`/`Country`; a TIFF carries the location in XMP only, and a place you set replaces the source file's GPS block whole, rather than leaving its altitude or heading beside your coordinates. A geotagged source with no place set here keeps its own coordinates on export, and the map opens centred on them — where the frame was digitized is a starting view, never the capture place. Opening the map contacts OpenStreetMap; typing coordinates needs no network. + **Process:** * **Format**: `35mm`, `120`, `4×5`, `8×10`, `110`, or `Other` with a free-text field. @@ -833,7 +840,7 @@ Archival metadata for the **original analog capture** (camera, lens, film, proce **Exposure**: optional original shutter, aperture and ISO. Click the lock to edit a free-text string, for example `1/125s f/2.8 ISO 400`. -**Metadata preview**: a live view of exactly what will be embedded, grouped by capture, scan, process and file. **Description…** opens a checklist of which fields join into EXIF `ImageDescription`. The defaults are camera, lens, film stock and ISO; format, developer, push/pull and scanning are off until you enable them. Confirming **Description…** sets that frame's selection and becomes the sticky default for other frames that do not have their own, so the last confirm on the roll wins. Sync metadata and Sync settings can also copy a frame's selection with the rest of the metadata. +**Metadata preview**: a live view of exactly what will be embedded, grouped by capture, scan, process and file. The Scan group shows the source file's own timestamp and coordinates, so you can see what you are replacing. **Description…** opens a checklist of which fields join into EXIF `ImageDescription`. The defaults are camera, lens, film stock and ISO; format, developer, push/pull and scanning are off until you enable them. Confirming **Description…** sets that frame's selection and becomes the sticky default for other frames that do not have their own, so the last confirm on the roll wins. Sync metadata and Sync settings can also copy a frame's selection with the rest of the metadata. When you set capture gear, it is written to standard EXIF, and the digitizing rig is preserved separately in `negpy:Scan*` XMP tags. Leave gear unset and your scanner or DSLR stays visible in EXIF instead. diff --git a/negpy/desktop/settings_catalog.py b/negpy/desktop/settings_catalog.py index 42ad28bc..be0966f7 100644 --- a/negpy/desktop/settings_catalog.py +++ b/negpy/desktop/settings_catalog.py @@ -14,6 +14,7 @@ from typing import Any, Callable, Iterable, Mapping, Optional from negpy.domain.models import WorkspaceConfig +from negpy.features.metadata.capture import place_summary from negpy.features.metadata.models import PUSH_PULL_LABELS from negpy.features.process.models import invalidate_local_bounds @@ -207,6 +208,17 @@ def _row(label, section, *fields, channels="", fmt=None) -> SettingRow: _row("Film Manufacturer", "metadata", "film_manufacturer"), _row("Film Color Type", "metadata", "film_color_type"), _row("Format", "metadata", "format", "format_other", fmt=lambda v: (v[1] if v[0] == "Other" and v[1] else v[0]) or "—"), + _row("Capture Date", "metadata", "capture_date"), + _row( + "Place", + "metadata", + "location_city", + "location_state", + "location_country", + "gps_latitude", + "gps_longitude", + fmt=lambda v: place_summary(v[0], v[1], v[2], v[3], v[4]) or "—", + ), _row("Developer", "metadata", "developer"), _row("Push/Pull", "metadata", "push_pull", fmt=lambda v: PUSH_PULL_LABELS.get(v[0], str(v[0]))), _row("Scanning", "metadata", "scanning"), diff --git a/negpy/desktop/view/sidebar/files.py b/negpy/desktop/view/sidebar/files.py index 01760cde..a69f7187 100644 --- a/negpy/desktop/view/sidebar/files.py +++ b/negpy/desktop/view/sidebar/files.py @@ -536,8 +536,8 @@ def _init_ui(self) -> None: self.search_input.setToolTip( "Filter the sheet. A bare word matches the filename; terms are combined with AND.\n" "Fields: film, camera, lens, developer, format, scanning, roll, frame, iso, push,\n" - "name, path, ext, date, keeper, rejected, edited.\n" - 'Examples: film:portra iso:>=400 · camera:"Nikon F3" -rejected: · date:>=2024-03' + "shot, place, name, path, ext, date, keeper, rejected, edited.\n" + 'Examples: film:portra iso:>=400 · camera:"Nikon F3" -rejected: · shot:>=1998-07 · place:tokyo' ) self.search_input.setClearButtonEnabled(True) self.search_input.addAction( diff --git a/negpy/desktop/view/sidebar/metadata.py b/negpy/desktop/view/sidebar/metadata.py index 68a6b359..4f6ec80c 100644 --- a/negpy/desktop/view/sidebar/metadata.py +++ b/negpy/desktop/view/sidebar/metadata.py @@ -1,5 +1,6 @@ import qtawesome as qta from dataclasses import asdict, replace +from typing import Optional from PyQt6.QtCore import QTimer from PyQt6.QtWidgets import ( QCheckBox, @@ -19,7 +20,15 @@ from negpy.desktop.view.widgets.collapsible import CollapsibleSection from negpy.desktop.view.widgets.description_fields_dialog import DescriptionFieldsDialog from negpy.desktop.view.widgets.gear_library_dialog import GearLibraryDialog +from negpy.desktop.view.widgets.location_picker_dialog import LocationPickerDialog from negpy.desktop.view.widgets.searchable_gear_combo import SearchableGearCombo +from negpy.features.metadata.capture import ( + CAPTURE_DATE_HINT, + parse_capture_date, + parse_coords, + place_summary, +) +from negpy.features.metadata.exif_read import extract_scan_from_exif from negpy.features.metadata.gear_logic import metadata_from_gear from negpy.features.metadata.gear_models import GearLibrary from negpy.features.metadata.models import DEFAULT_DESCRIPTION_FIELDS @@ -103,6 +112,33 @@ def _init_ui(self) -> None: gear.addWidget(self.manage_btn) controls.addWidget(self._card("Analog Gear", "gear", gear_body, "fa5s.camera-retro")) + # ── CAPTURE ────────────────────────────────────────────────────── + cap_body, cap = self._card_body() + cap.addWidget(field_label("Date")) + self.capture_date_edit = QLineEdit() + self.capture_date_edit.setPlaceholderText(CAPTURE_DATE_HINT) + self.capture_date_edit.setText(conf.capture_date) + self.capture_date_edit.setToolTip( + "When the frame was shot. Give only what you know: a year, a year and month, " + "a date, or a date and time. An offset like +02:00 may follow a time." + ) + cap.addWidget(self.capture_date_edit) + + cap.addWidget(field_label("Place")) + place_row = QHBoxLayout() + place_row.setSpacing(THEME.space_sm) + self.place_edit = QLineEdit() + self.place_edit.setPlaceholderText("Pick on a map, or paste coordinates") + self.place_edit.setToolTip("Capture place. Paste a coordinate pair or a map link here, or use Map… to pick one.") + place_row.addWidget(self.place_edit, 1) + self.place_map_btn = QPushButton("Map…") + self.place_map_btn.setToolTip("Pick the capture place on a map (contacts OpenStreetMap)") + place_row.addWidget(self.place_map_btn) + self.place_clear_btn = QPushButton("Clear") + place_row.addWidget(self.place_clear_btn) + cap.addLayout(place_row) + controls.addWidget(self._card("Capture", "capture", cap_body, "fa5s.clock")) + # ── PROCESS ────────────────────────────────────────────────────── proc_body, proc = self._card_body() proc.addWidget(field_label("Format")) @@ -285,6 +321,11 @@ def _connect_signals(self) -> None: self.film_stock_combo.selection_changed.connect(self._on_gear_changed) self.manage_btn.clicked.connect(self._open_gear_library) + self.capture_date_edit.textChanged.connect(self._on_capture_date_changed) + self.place_edit.editingFinished.connect(self._on_place_edited) + self.place_map_btn.clicked.connect(self._open_location_picker) + self.place_clear_btn.clicked.connect(self._on_place_clear) + self.format_combo.currentTextChanged.connect(self._on_format_changed) self.format_other_edit.textChanged.connect(self._mark_dirty) self.developer_edit.textChanged.connect(self._mark_dirty) @@ -456,6 +497,85 @@ def _mark_dirty(self) -> None: def _schedule_preview(self) -> None: self.preview_timer.start() + def _on_capture_date_changed(self, text: str) -> None: + valid = not text.strip() or parse_capture_date(text) is not None + self.capture_date_edit.setStyleSheet("" if valid else f"border: 1px solid {THEME.accent_secondary};") + self._mark_dirty() + + def _source_exif(self) -> Optional[dict]: + current_hash = self.state.current_file_hash + if current_hash and current_hash in self.state.source_exif: + return self.state.source_exif[current_hash] + return None + + def _place_text(self) -> str: + conf = self.state.config.metadata + return place_summary( + conf.location_city, + conf.location_state, + conf.location_country, + conf.gps_latitude, + conf.gps_longitude, + ) + + def _apply_location(self, lat, lon, city: str, state: str, country: str) -> None: + self.update_config_section( + "metadata", + persist=True, + render=False, + readback_metrics=False, + gps_latitude=lat, + gps_longitude=lon, + location_city=city, + location_state=state, + location_country=country, + ) + self._set_place_text_quiet() + self._schedule_preview() + + def _set_place_text_quiet(self) -> None: + self.place_edit.blockSignals(True) + try: + self.place_edit.setText(self._place_text()) + finally: + self.place_edit.blockSignals(False) + + def _on_place_edited(self) -> None: + """Typed text is only ever coordinates; place names come from the picker.""" + text = self.place_edit.text().strip() + if not text: + self._on_place_clear() + return + coords = parse_coords(text) + if coords is None: + self._set_place_text_quiet() + return + conf = self.state.config.metadata + self._apply_location(coords[0], coords[1], conf.location_city, conf.location_state, conf.location_country) + + def _on_place_clear(self) -> None: + self._apply_location(None, None, "", "", "") + + def _open_location_picker(self) -> None: + conf = self.state.config.metadata + center = None + if conf.gps_latitude is None or conf.gps_longitude is None: + scan = extract_scan_from_exif(self._source_exif()) + if scan.gps_latitude is not None and scan.gps_longitude is not None: + center = (scan.gps_latitude, scan.gps_longitude) + dlg = LocationPickerDialog( + conf.gps_latitude, + conf.gps_longitude, + conf.location_city, + conf.location_state, + conf.location_country, + center=center, + parent=self, + ) + if dlg.exec() != dlg.DialogCode.Accepted: + return + self._apply_location(*dlg.location()) + def _on_format_changed(self, text: str) -> None: self.format_other_edit.setVisible(text == "Other") self._mark_dirty() @@ -480,11 +600,16 @@ def _persist_all_metadata_settings(self) -> None: except ValueError: capture_frame = self.state.config.metadata.capture_frame + date_text = self.capture_date_edit.text().strip() + parsed_date = parse_capture_date(date_text) + capture_date = parsed_date.xmp_text() if parsed_date else ("" if not date_text else self.state.config.metadata.capture_date) + self.update_config_section( "metadata", persist=True, render=False, readback_metrics=False, + capture_date=capture_date, gear_preset_id=self.preset_combo.selected_id(), camera_id=self.camera_combo.selected_id(), lens_id=self.lens_combo.selected_id(), @@ -518,6 +643,9 @@ def sync_ui(self) -> None: self.format_combo.setCurrentText("Other") self.format_other_edit.setText(conf.format_other) self.format_other_edit.setVisible(self.format_combo.currentText() == "Other") + self.capture_date_edit.setText(conf.capture_date) + self.capture_date_edit.setStyleSheet("") + self.place_edit.setText(self._place_text()) self.developer_edit.setText(conf.developer) idx = PUSH_PULL_VALUES.index(conf.push_pull) if conf.push_pull in PUSH_PULL_VALUES else 3 self.push_pull_combo.setCurrentIndex(idx) @@ -579,8 +707,11 @@ def _preview_metadata_config(self): else: exposure_override = conf.exposure_override + parsed_date = parse_capture_date(self.capture_date_edit.text()) + return replace( conf, + capture_date=parsed_date.xmp_text() if parsed_date else "", gear_preset_id=self.preset_combo.selected_id(), camera_id=self.camera_combo.selected_id(), lens_id=self.lens_combo.selected_id(), @@ -608,12 +739,7 @@ def _update_preview(self) -> None: self.preview_section.setEnabled(True) return - source_exif = None - current_hash = self.state.current_file_hash - if current_hash and current_hash in self.state.source_exif: - source_exif = self.state.source_exif[current_hash] - - payload = build_metadata_payload(self._preview_metadata_config(), self._gear_library, source_exif) + payload = build_metadata_payload(self._preview_metadata_config(), self._gear_library, self._source_exif()) sections = payload.to_preview_sections() self.preview_empty.setText("Select gear or enter process metadata to see a preview.") diff --git a/negpy/desktop/view/widgets/location_picker_dialog.py b/negpy/desktop/view/widgets/location_picker_dialog.py new file mode 100644 index 00000000..3befc616 --- /dev/null +++ b/negpy/desktop/view/widgets/location_picker_dialog.py @@ -0,0 +1,247 @@ +"""Pick the capture place on a map, by place name, or by typed coordinates.""" + +from __future__ import annotations + +from typing import Optional + +from PyQt6.QtCore import QObject, QRunnable, QThreadPool, pyqtSignal +from PyQt6.QtWidgets import ( + QDialog, + QDialogButtonBox, + QGridLayout, + QHBoxLayout, + QLineEdit, + QListWidget, + QPushButton, + QVBoxLayout, +) + +from negpy.desktop.view.styles.templates import field_label, hint_label +from negpy.desktop.view.styles.theme import THEME +from negpy.desktop.view.widgets.slippy_map import SlippyMapWidget +from negpy.features.metadata.capture import format_coords, parse_coords +from negpy.services.maps import place_fields, result_coords, reverse_place, search_places + +_OFFLINE_HINT = "Map unavailable — enter coordinates manually." +_SHUTDOWN_WAIT_MS = 6000 + + +class _LookupSignals(QObject): + search_done = pyqtSignal(object) + reverse_done = pyqtSignal(int, object) + + def __init__(self): + super().__init__() + self.stopped = False + + +class _SearchJob(QRunnable): + def __init__(self, signals: _LookupSignals, query: str): + super().__init__() + self._signals = signals + self._query = query + + def run(self) -> None: + if self._signals.stopped: + return + self._signals.search_done.emit(search_places(self._query)) + + +class _ReverseJob(QRunnable): + def __init__(self, signals: _LookupSignals, token: int, lat: float, lon: float): + super().__init__() + self._signals = signals + self._token = token + self._lat, self._lon = lat, lon + + def run(self) -> None: + if self._signals.stopped: + return + self._signals.reverse_done.emit(self._token, reverse_place(self._lat, self._lon)) + + +class LocationPickerDialog(QDialog): + """Coordinates are authoritative; the place names are a proposal the user can edit.""" + + def __init__( + self, + lat: Optional[float] = None, + lon: Optional[float] = None, + city: str = "", + state: str = "", + country: str = "", + center: Optional[tuple[float, float]] = None, + parent=None, + ): + super().__init__(parent) + self.setWindowTitle("Capture location") + self.setMinimumSize(560, 560) + self.setStyleSheet(f"QDialog {{ background: {THEME.bg_dark}; }}") + + # The pool is owned by the dialog, so closing it joins any running lookup before the + # signal object goes away. done() drops the queue first to keep that join short. + self._pool = QThreadPool(self) + self._pool.setMaxThreadCount(2) + self._signals = _LookupSignals() + self._signals.search_done.connect(self._on_search_done) + self._signals.reverse_done.connect(self._on_reverse_done) + self._reverse_token = 0 + self._results: list[dict] = [] + + root = QVBoxLayout(self) + root.setContentsMargins(THEME.space_xl, THEME.space_xl, THEME.space_xl, THEME.space_xl) + root.setSpacing(THEME.space_lg) + + root.addWidget( + hint_label("Search a place, click the map, or paste coordinates or a map link. Opening this dialog contacts OpenStreetMap.") + ) + + search_row = QHBoxLayout() + search_row.setSpacing(THEME.space_sm) + self.search_edit = QLineEdit() + self.search_edit.setPlaceholderText("e.g. Tokyo, Japan") + self.search_edit.returnPressed.connect(self._on_search) + self.search_btn = QPushButton("Search") + self.search_btn.clicked.connect(self._on_search) + search_row.addWidget(self.search_edit, 1) + search_row.addWidget(self.search_btn) + root.addLayout(search_row) + + self.results_list = QListWidget() + self.results_list.setMaximumHeight(96) + self.results_list.setVisible(False) + self.results_list.currentRowChanged.connect(self._on_result_selected) + root.addWidget(self.results_list) + + self.map_view = SlippyMapWidget() + self.map_view.pin_moved.connect(self._on_pin_moved) + root.addWidget(self.map_view, 1) + + fields = QGridLayout() + fields.setHorizontalSpacing(THEME.space_lg) + fields.setVerticalSpacing(THEME.space_sm) + + self.coords_edit = QLineEdit() + self.coords_edit.setPlaceholderText("35.67620, 139.65030") + self.coords_edit.editingFinished.connect(self._on_coords_edited) + self.city_edit = QLineEdit(city) + self.state_edit = QLineEdit(state) + self.country_edit = QLineEdit(country) + + for column, (label, widget) in enumerate( + ( + ("Coordinates", self.coords_edit), + ("City", self.city_edit), + ("State", self.state_edit), + ("Country", self.country_edit), + ) + ): + fields.addWidget(field_label(label), (column // 2) * 2, column % 2) + fields.addWidget(widget, (column // 2) * 2 + 1, column % 2) + root.addLayout(fields) + + self.status_label = hint_label("") + root.addWidget(self.status_label) + + buttons = QDialogButtonBox(QDialogButtonBox.StandardButton.Ok | QDialogButtonBox.StandardButton.Cancel) + buttons.accepted.connect(self.accept) + buttons.rejected.connect(self.reject) + root.addWidget(buttons) + + if lat is not None and lon is not None: + self.coords_edit.setText(format_coords(lat, lon)) + self.map_view.set_pin(lat, lon) + self.map_view.set_zoom(10) + elif center is not None: + # The scan file's own position only frames the view. Adopting it as the capture + # place would claim the frame was shot where it was digitized. + self.map_view.set_center(*center) + self.map_view.set_zoom(8) + self.status_label.setText("Centred on the scan file's coordinates.") + + def done(self, result: int) -> None: + # Join the lookup threads here: the pool's destructor would wait with the GIL held, + # which a worker needs to finish, and the app would hang instead of closing. + self._signals.stopped = True + self._pool.clear() + self._pool.waitForDone(_SHUTDOWN_WAIT_MS) + self.map_view.shutdown() + super().done(result) + + def location(self) -> tuple[Optional[float], Optional[float], str, str, str]: + coords = parse_coords(self.coords_edit.text()) + lat, lon = coords if coords else (None, None) + return ( + lat, + lon, + self.city_edit.text().strip(), + self.state_edit.text().strip(), + self.country_edit.text().strip(), + ) + + # ── search ─────────────────────────────────────────────────────────── + + def _on_search(self) -> None: + query = self.search_edit.text().strip() + if not query: + return + self.status_label.setText("Searching…") + self._pool.start(_SearchJob(self._signals, query)) + + def _on_search_done(self, results: object) -> None: + self._results = list(results) if isinstance(results, list) else [] + self.results_list.blockSignals(True) + self.results_list.clear() + for item in self._results: + self.results_list.addItem(str(item.get("display_name", ""))) + self.results_list.setCurrentRow(-1) + self.results_list.blockSignals(False) + self.results_list.setVisible(bool(self._results)) + self.status_label.setText("" if self._results else _OFFLINE_HINT) + + def _on_result_selected(self, row: int) -> None: + if not 0 <= row < len(self._results): + return + result = self._results[row] + coords = result_coords(result) + if coords is None: + return + self.coords_edit.setText(format_coords(*coords)) + self.map_view.set_pin(*coords) + self.map_view.set_zoom(10) + self._apply_place(result) + + # ── map and coordinates ────────────────────────────────────────────── + + def _on_pin_moved(self, lat: float, lon: float) -> None: + self.coords_edit.setText(format_coords(lat, lon)) + self._start_reverse(lat, lon) + + def _on_coords_edited(self) -> None: + coords = parse_coords(self.coords_edit.text()) + if coords is None: + self.status_label.setText("Coordinates not recognised.") + return + self.coords_edit.setText(format_coords(*coords)) + self.map_view.set_pin(*coords) + self._start_reverse(*coords) + + def _start_reverse(self, lat: float, lon: float) -> None: + self._reverse_token += 1 + self.status_label.setText("Looking up place…") + self._pool.start(_ReverseJob(self._signals, self._reverse_token, lat, lon)) + + def _on_reverse_done(self, token: int, result: object) -> None: + if token != self._reverse_token: + return + if not isinstance(result, dict): + self.status_label.setText(_OFFLINE_HINT) + return + self._apply_place(result) + self.status_label.setText("") + + def _apply_place(self, result: dict) -> None: + city, state, country = place_fields(result) + self.city_edit.setText(city) + self.state_edit.setText(state) + self.country_edit.setText(country) diff --git a/negpy/desktop/view/widgets/slippy_map.py b/negpy/desktop/view/widgets/slippy_map.py new file mode 100644 index 00000000..0cbb596b --- /dev/null +++ b/negpy/desktop/view/widgets/slippy_map.py @@ -0,0 +1,261 @@ +"""A pan/zoom OpenStreetMap tile view with one draggable pin.""" + +from __future__ import annotations + +import math +from typing import Optional + +from PyQt6.QtCore import QObject, QPoint, QRunnable, Qt, QThreadPool, pyqtSignal +from PyQt6.QtGui import QColor, QFont, QMouseEvent, QPainter, QPixmap, QWheelEvent +from PyQt6.QtWidgets import QWidget + +from negpy.desktop.view.styles.theme import THEME +from negpy.features.metadata.capture import deg2tile, tile2deg +from negpy.services.maps import MAX_ZOOM, MIN_ZOOM, TILE_SIZE, fetch_tile + +_ATTRIBUTION = "© OpenStreetMap contributors" +_DRAG_SLOP_PX = 4 +_MAX_CONCURRENT_TILES = 4 +# Panning enqueues tiles faster than they arrive, and the pool joins its queue when the view +# closes. Cap the queue so that join is short, and so stale requests cannot pile up. +_MAX_PENDING_TILES = 24 +_SHUTDOWN_WAIT_MS = 6000 + + +class _TileSignals(QObject): + ready = pyqtSignal(int, int, int, object) + + def __init__(self): + super().__init__() + self.stopped = False + + +class _TileJob(QRunnable): + def __init__(self, signals: _TileSignals, z: int, x: int, y: int): + super().__init__() + self._signals = signals + self._key = (z, x, y) + + def run(self) -> None: + if self._signals.stopped: + return + data = fetch_tile(*self._key) + if self._signals.stopped: + return + self._signals.ready.emit(*self._key, data) + + +class SlippyMapWidget(QWidget): + """Tiles are fetched off the GUI thread; a missing tile paints flat and nothing blocks.""" + + pin_moved = pyqtSignal(float, float) + + def __init__(self, parent=None): + super().__init__(parent) + self.setMinimumSize(420, 300) + self.setCursor(Qt.CursorShape.CrossCursor) + + self._zoom = 4 + self._center = (50.0, 15.0) + self._pin: Optional[tuple[float, float]] = None + + self._tiles: dict[tuple[int, int, int], QPixmap] = {} + self._requested: set[tuple[int, int, int]] = set() + self._missing: set[tuple[int, int, int]] = set() + + self._pool = QThreadPool(self) + self._pool.setMaxThreadCount(_MAX_CONCURRENT_TILES) + self._signals = _TileSignals() + self._signals.ready.connect(self._on_tile_ready) + + self._drag_origin: Optional[QPoint] = None + self._dragged = False + + # ── state ──────────────────────────────────────────────────────────── + + def pin(self) -> Optional[tuple[float, float]]: + return self._pin + + def set_pin(self, lat: float, lon: float, *, recenter: bool = True) -> None: + self._pin = (lat, lon) + if recenter: + self._center = (lat, lon) + self.update() + + def set_center(self, lat: float, lon: float) -> None: + self._center = (lat, lon) + self.update() + + def clear_pin(self) -> None: + self._pin = None + self.update() + + def set_zoom(self, zoom: int) -> None: + self._zoom = max(MIN_ZOOM, min(MAX_ZOOM, zoom)) + self.update() + + # ── coordinate helpers ─────────────────────────────────────────────── + + def _tile_at(self, x_px: float, y_px: float) -> tuple[float, float]: + cx, cy = deg2tile(*self._center, self._zoom) + return ( + cx + (x_px - self.width() / 2.0) / TILE_SIZE, + cy + (y_px - self.height() / 2.0) / TILE_SIZE, + ) + + def _pixel_at(self, lat: float, lon: float) -> tuple[float, float]: + cx, cy = deg2tile(*self._center, self._zoom) + tx, ty = deg2tile(lat, lon, self._zoom) + return ( + self.width() / 2.0 + (tx - cx) * TILE_SIZE, + self.height() / 2.0 + (ty - cy) * TILE_SIZE, + ) + + def latlon_at(self, x_px: float, y_px: float) -> tuple[float, float]: + return tile2deg(*self._tile_at(x_px, y_px), self._zoom) + + # ── tiles ──────────────────────────────────────────────────────────── + + def _request(self, key: tuple[int, int, int]) -> None: + if self._signals.stopped or len(self._requested) >= _MAX_PENDING_TILES: + return + if key in self._tiles or key in self._requested or key in self._missing: + return + self._requested.add(key) + self._pool.start(_TileJob(self._signals, *key)) + + def shutdown(self) -> None: + """ + Drop pending tiles and join the running ones here, not in the pool's destructor: that + destructor waits while holding the GIL, so a fetch thread could never finish and the + GUI would hang for good. waitForDone releases the GIL, so the wait is one fetch long. + """ + self._signals.stopped = True + self._pool.clear() + self._pool.waitForDone(_SHUTDOWN_WAIT_MS) + + def hideEvent(self, event) -> None: # noqa: N802 - Qt override + self.shutdown() + super().hideEvent(event) + + def showEvent(self, event) -> None: # noqa: N802 - Qt override + self._signals.stopped = False + super().showEvent(event) + + def _on_tile_ready(self, z: int, x: int, y: int, data: object) -> None: + if self._signals.stopped: + return + key = (z, x, y) + self._requested.discard(key) + pixmap = QPixmap() + if isinstance(data, (bytes, bytearray)) and pixmap.loadFromData(bytes(data)): + self._tiles[key] = pixmap + else: + self._missing.add(key) + self.update() + + # ── painting ───────────────────────────────────────────────────────── + + def paintEvent(self, event) -> None: # noqa: N802 - Qt override + painter = QPainter(self) + painter.fillRect(self.rect(), QColor(THEME.canvas_bg_dark_grey)) + + span = 2**self._zoom + left, top = self._tile_at(0.0, 0.0) + first_x, first_y = math.floor(left), math.floor(top) + offset_x = (first_x - left) * TILE_SIZE + offset_y = (first_y - top) * TILE_SIZE + + columns = int(self.width() / TILE_SIZE) + 2 + rows = int(self.height() / TILE_SIZE) + 2 + + for col in range(columns): + for row in range(rows): + tile_x, tile_y = first_x + col, first_y + row + if not 0 <= tile_y < span: + continue + key = (self._zoom, tile_x % span, tile_y) + px = int(offset_x + col * TILE_SIZE) + py = int(offset_y + row * TILE_SIZE) + pixmap = self._tiles.get(key) + if pixmap is None: + self._request(key) + painter.fillRect(px, py, TILE_SIZE, TILE_SIZE, QColor(THEME.canvas_bg_mid_grey)) + continue + painter.drawPixmap(px, py, pixmap) + + self._paint_pin(painter) + self._paint_attribution(painter) + painter.end() + + def _paint_pin(self, painter: QPainter) -> None: + if self._pin is None: + return + x, y = self._pixel_at(*self._pin) + painter.setPen(QColor(THEME.accent_secondary)) + painter.setBrush(QColor(THEME.accent_primary)) + painter.drawEllipse(int(x) - 5, int(y) - 5, 10, 10) + painter.drawLine(int(x), int(y) - 14, int(x), int(y) - 5) + + def _paint_attribution(self, painter: QPainter) -> None: + font = QFont(painter.font()) + font.setPointSize(8) + painter.setFont(font) + metrics = painter.fontMetrics() + width = metrics.horizontalAdvance(_ATTRIBUTION) + 8 + height = metrics.height() + 2 + painter.fillRect(self.width() - width, self.height() - height, width, height, QColor(0, 0, 0, 150)) + painter.setPen(QColor(THEME.text_primary)) + painter.drawText(self.width() - width + 4, self.height() - 3 - metrics.descent(), _ATTRIBUTION) + + # ── interaction ────────────────────────────────────────────────────── + + def mousePressEvent(self, event: QMouseEvent) -> None: # noqa: N802 - Qt override + if event.button() != Qt.MouseButton.LeftButton: + return + self._drag_origin = event.pos() + self._dragged = False + + def mouseMoveEvent(self, event: QMouseEvent) -> None: # noqa: N802 - Qt override + if self._drag_origin is None: + return + delta = event.pos() - self._drag_origin + if not self._dragged and delta.manhattanLength() < _DRAG_SLOP_PX: + return + self._dragged = True + self._drag_origin = event.pos() + cx, cy = deg2tile(*self._center, self._zoom) + self._center = tile2deg(cx - delta.x() / TILE_SIZE, cy - delta.y() / TILE_SIZE, self._zoom) + self.update() + + def mouseReleaseEvent(self, event: QMouseEvent) -> None: # noqa: N802 - Qt override + if self._drag_origin is None: + return + was_drag = self._dragged + self._drag_origin = None + self._dragged = False + if was_drag: + return + lat, lon = self.latlon_at(event.pos().x(), event.pos().y()) + self.set_pin(lat, lon, recenter=False) + self.pin_moved.emit(lat, lon) + + def wheelEvent(self, event: QWheelEvent) -> None: # noqa: N802 - Qt override + notches = event.angleDelta().y() + if not notches: + return + step = 1 if notches > 0 else -1 + zoom = max(MIN_ZOOM, min(MAX_ZOOM, self._zoom + step)) + if zoom == self._zoom: + return + # Keep the position under the cursor fixed, so the wheel zooms into what is aimed at. + pos = event.position() + anchor = self.latlon_at(pos.x(), pos.y()) + self._zoom = zoom + ax, ay = deg2tile(*anchor, zoom) + self._center = tile2deg( + ax - (pos.x() - self.width() / 2.0) / TILE_SIZE, + ay - (pos.y() - self.height() / 2.0) / TILE_SIZE, + zoom, + ) + self.update() diff --git a/negpy/features/metadata/capture.py b/negpy/features/metadata/capture.py new file mode 100644 index 00000000..2c71c5ce --- /dev/null +++ b/negpy/features/metadata/capture.py @@ -0,0 +1,205 @@ +"""Capture time and place: parse, validate, and convert for EXIF/XMP.""" + +from __future__ import annotations + +import math +import re +from dataclasses import dataclass +from datetime import datetime +from typing import Optional + +import piexif + +PRECISIONS = ("year", "month", "day", "minute", "second") + +CAPTURE_DATE_HINT = "YYYY, YYYY-MM, YYYY-MM-DD or YYYY-MM-DD HH:MM" + +_DATE_RE = re.compile( + r"^(?P\d{4})" + r"(?:-(?P\d{1,2})" + r"(?:-(?P\d{1,2})" + r"(?:[ T](?P\d{1,2}):(?P\d{2})" + r"(?::(?P\d{2}))?" + r"\s*(?PZ|[+-]\d{2}:?\d{2})?)?)?)?$" +) + +_MIN_YEAR = 1800 +_MAX_YEAR = 2999 + + +@dataclass(frozen=True) +class CaptureDate: + """A capture instant known only to some precision. `text` is ISO-8601, truncated.""" + + text: str + precision: str + tz_offset: str = "" + + @property + def year(self) -> int: + return int(self.text[:4]) + + def xmp_text(self) -> str: + return f"{self.text}{self.tz_offset}" + + def exif_text(self) -> str: + """EXIF cannot hold a partial date, so the unknown parts are padded.""" + y, mo, d, h, mi, s = _parts(self) + return f"{y:04d}:{mo:02d}:{d:02d} {h:02d}:{mi:02d}:{s:02d}" + + def compact(self) -> str: + y, mo, d, _h, _mi, _s = _parts(self) + return f"{y:04d}{mo:02d}{d:02d}" + + +def _parts(cd: CaptureDate) -> tuple[int, int, int, int, int, int]: + m = _DATE_RE.match(cd.text) + if m is None: + return int(cd.text[:4]), 1, 1, 0, 0, 0 + + def num(key: str, default: int) -> int: + raw = m.group(key) + return int(raw) if raw else default + + return num("y", 1), num("mo", 1), num("d", 1), num("h", 0), num("mi", 0), num("s", 0) + + +def _normalize_offset(raw: str) -> str: + if raw == "Z": + return "+00:00" + body = raw.replace(":", "") + return f"{body[:3]}:{body[3:]}" + + +def parse_capture_date(text: str) -> Optional[CaptureDate]: + """A CaptureDate from a truncated ISO-8601 string, or None when it is not a real instant.""" + cleaned = text.strip().replace("/", "-") + if not cleaned: + return None + + m = _DATE_RE.match(cleaned) + if m is None: + return None + + year = int(m.group("y")) + if not _MIN_YEAR <= year <= _MAX_YEAR: + return None + + month, day = m.group("mo"), m.group("d") + hour, minute, second = m.group("h"), m.group("mi"), m.group("s") + + try: + datetime( + year, + int(month) if month else 1, + int(day) if day else 1, + int(hour) if hour else 0, + int(minute) if minute else 0, + int(second) if second else 0, + ) + except ValueError: + return None + + normalized = f"{year:04d}" + precision = "year" + if month: + normalized += f"-{int(month):02d}" + precision = "month" + if day: + normalized += f"-{int(day):02d}" + precision = "day" + if hour: + normalized += f" {int(hour):02d}:{minute}" + precision = "minute" + if second: + normalized += f":{second}" + precision = "second" + + tz = m.group("tz") + return CaptureDate(normalized, precision, _normalize_offset(tz) if tz and hour else "") + + +_OSM_HASH_RE = re.compile(r"map=\d+(?:\.\d+)?/(-?\d+(?:\.\d+)?)/(-?\d+(?:\.\d+)?)") +_MARKER_RE = re.compile(r"mlat=(-?\d+(?:\.\d+)?)[^0-9-]+mlon=(-?\d+(?:\.\d+)?)") +_AT_RE = re.compile(r"[@=](-?\d+(?:\.\d+)?),\s*(-?\d+(?:\.\d+)?)") +_PAIR_RE = re.compile(r"^\s*(-?\d+(?:\.\d+)?)\s*[,; ]\s*(-?\d+(?:\.\d+)?)\s*$") + + +def parse_coords(text: str) -> Optional[tuple[float, float]]: + """Latitude/longitude from a typed pair or a pasted OpenStreetMap / Google Maps link.""" + stripped = text.strip() + if not stripped: + return None + + for pattern in (_OSM_HASH_RE, _MARKER_RE, _AT_RE, _PAIR_RE): + m = pattern.search(stripped) + if m is None: + continue + lat, lon = float(m.group(1)), float(m.group(2)) + if abs(lat) <= 90.0 and abs(lon) <= 180.0: + return lat, lon + return None + + +def format_coords(lat: float, lon: float) -> str: + return f"{lat:.5f}, {lon:.5f}" + + +def place_summary(city: str, state: str, country: str, lat: Optional[float], lon: Optional[float]) -> str: + """One-line place label: names when known, otherwise the coordinates.""" + names = [part.strip() for part in (city, state, country) if part.strip()] + if names: + return ", ".join(names) + if lat is not None and lon is not None: + return format_coords(lat, lon) + return "" + + +def _dms(value: float) -> tuple[tuple[int, int], tuple[int, int], tuple[int, int]]: + total = abs(value) + degrees = int(total) + minutes_full = (total - degrees) * 60.0 + minutes = int(minutes_full) + seconds = round((minutes_full - minutes) * 60.0 * 100.0) + return (degrees, 1), (minutes, 1), (int(seconds), 100) + + +def exif_gps_rationals(lat: float, lon: float) -> dict: + """A piexif GPS IFD for a WGS-84 position.""" + return { + piexif.GPSIFD.GPSVersionID: (2, 3, 0, 0), + piexif.GPSIFD.GPSLatitudeRef: b"N" if lat >= 0 else b"S", + piexif.GPSIFD.GPSLatitude: _dms(lat), + piexif.GPSIFD.GPSLongitudeRef: b"E" if lon >= 0 else b"W", + piexif.GPSIFD.GPSLongitude: _dms(lon), + piexif.GPSIFD.GPSMapDatum: b"WGS-84", + } + + +def xmp_gps(lat: float, lon: float) -> tuple[str, str]: + """XMP exif:GPSLatitude / exif:GPSLongitude in the spec's `DDD,MM.mmK` form.""" + + def one(value: float, positive: str, negative: str) -> str: + total = abs(value) + degrees = int(total) + minutes = (total - degrees) * 60.0 + return f"{degrees},{minutes:.4f}{positive if value >= 0 else negative}" + + return one(lat, "N", "S"), one(lon, "E", "W") + + +def deg2tile(lat: float, lon: float, zoom: int) -> tuple[float, float]: + """Fractional Web Mercator tile coordinates.""" + lat = max(-85.05112878, min(85.05112878, lat)) + n = float(2**zoom) + x = (lon + 180.0) / 360.0 * n + rad = math.radians(lat) + y = (1.0 - math.asinh(math.tan(rad)) / math.pi) / 2.0 * n + return x, min(max(y, 0.0), n) + + +def tile2deg(x: float, y: float, zoom: int) -> tuple[float, float]: + n = float(2**zoom) + lon = x / n * 360.0 - 180.0 + lat = math.degrees(math.atan(math.sinh(math.pi * (1.0 - 2.0 * y / n)))) + return lat, lon diff --git a/negpy/features/metadata/exif_read.py b/negpy/features/metadata/exif_read.py index 51dc9b11..34381e71 100644 --- a/negpy/features/metadata/exif_read.py +++ b/negpy/features/metadata/exif_read.py @@ -61,6 +61,25 @@ def format_exposure(exif_tags: dict) -> str: return " ".join(parts) +def format_exif_datetime(value: Any) -> str: + """`YYYY:MM:DD HH:MM:SS` as read from EXIF, shown with ISO date separators.""" + text = safe_str(value).strip() + if len(text) >= 10 and text[4] == ":" and text[7] == ":": + return f"{text[:4]}-{text[5:7]}-{text[8:]}" + return text + + +def gps_decimal(dms: Any, ref: Any) -> Optional[float]: + """Signed decimal degrees from an EXIF GPS DMS triplet and its hemisphere reference.""" + if not isinstance(dms, (tuple, list)) or len(dms) != 3: + return None + parts = [rational_to_float(part) for part in dms] + if any(part is None for part in parts): + return None + value = parts[0] + parts[1] / 60.0 + parts[2] / 3600.0 + return -value if safe_str(ref).upper() in ("S", "W") else value + + @dataclass(frozen=True) class ScanExif: """DSLR / scanner rig metadata read from the source file before export overwrite.""" @@ -73,6 +92,9 @@ class ScanExif: aperture: Optional[float] = None iso: Optional[int] = None exposure: str = "" + datetime_original: str = "" + gps_latitude: Optional[float] = None + gps_longitude: Optional[float] = None def has_any(self) -> bool: return bool( @@ -84,6 +106,8 @@ def has_any(self) -> bool: or self.aperture is not None or self.iso is not None or self.exposure + or self.datetime_original + or self.gps_latitude is not None ) @@ -99,6 +123,14 @@ def has_any(self) -> bool: } ) + +def strip_scan_gps(exif_dict: dict) -> None: + """Drop the source GPS block whole: a picked location must not keep the scan's altitude, + timestamp or heading beside its own coordinates.""" + if isinstance(exif_dict.get("GPS"), dict): + exif_dict["GPS"] = {} + + _SCAN_EXPOSURE_EXIF_TAGS = frozenset( { piexif.ExifIFD.ExposureTime, @@ -125,6 +157,7 @@ def extract_scan_from_exif(source_exif: dict | None) -> ScanExif: zeroth = source_exif.get("0th", {}) or {} exif_tags = source_exif.get("Exif", {}) or {} + gps_tags = source_exif.get("GPS", {}) or {} iso_raw = exif_tags.get(piexif.ExifIFD.ISOSpeedRatings) iso: Optional[int] = None @@ -142,4 +175,7 @@ def extract_scan_from_exif(source_exif: dict | None) -> ScanExif: aperture=rational_to_float(exif_tags.get(piexif.ExifIFD.FNumber)), iso=iso, exposure=format_exposure(exif_tags), + datetime_original=format_exif_datetime(exif_tags.get(piexif.ExifIFD.DateTimeOriginal) or zeroth.get(piexif.ImageIFD.DateTime, "")), + gps_latitude=gps_decimal(gps_tags.get(piexif.GPSIFD.GPSLatitude), gps_tags.get(piexif.GPSIFD.GPSLatitudeRef)), + gps_longitude=gps_decimal(gps_tags.get(piexif.GPSIFD.GPSLongitude), gps_tags.get(piexif.GPSIFD.GPSLongitudeRef)), ) diff --git a/negpy/features/metadata/models.py b/negpy/features/metadata/models.py index 41721ffd..375b92d8 100644 --- a/negpy/features/metadata/models.py +++ b/negpy/features/metadata/models.py @@ -93,6 +93,16 @@ class MetadataConfig: scanning: str = "" sync_to_batch: bool = False + # Original capture instant, ISO-8601 truncated to the precision the user knows. + capture_date: str = "" + + # Capture place: WGS-84 position and the place names for it. + gps_latitude: Optional[float] = None + gps_longitude: Optional[float] = None + location_city: str = "" + location_state: str = "" + location_country: str = "" + # Scanlight capture identity (not process.roll_name / Roll Analysis) capture_roll: str = "" capture_frame: Optional[int] = None diff --git a/negpy/features/metadata/payload.py b/negpy/features/metadata/payload.py index dccd231b..528f5b9a 100644 --- a/negpy/features/metadata/payload.py +++ b/negpy/features/metadata/payload.py @@ -6,6 +6,7 @@ from dataclasses import dataclass from typing import Any, Optional +from negpy.features.metadata.capture import CaptureDate, format_coords, parse_capture_date, place_summary from negpy.features.metadata.exif_read import ScanExif, extract_scan_from_exif from negpy.features.metadata.gear_models import GearLibrary from negpy.features.metadata.models import ( @@ -87,6 +88,12 @@ class MetadataPayload: capture_exposure: str = "" capture_roll: str = "" capture_frame: Optional[int] = None + capture_date: Optional[CaptureDate] = None + gps_latitude: Optional[float] = None + gps_longitude: Optional[float] = None + location_city: str = "" + location_state: str = "" + location_country: str = "" # Digitization rig (negpy:Scan* XMP only; source EXIF when capture gear not set) scan_camera_make: str = "" @@ -98,6 +105,9 @@ class MetadataPayload: scan_iso: Optional[int] = None scan_exposure: str = "" scan_method: str = "" + scan_datetime: str = "" + scan_gps_latitude: Optional[float] = None + scan_gps_longitude: Optional[float] = None image_description: str = "" developer: str = "" @@ -114,11 +124,25 @@ def lens_display(self) -> str: def scan_camera_display(self) -> str: return f"{self.scan_camera_make} {self.scan_camera_model}".strip() + def place_display(self) -> str: + return place_summary( + self.location_city, + self.location_state, + self.location_country, + self.gps_latitude, + self.gps_longitude, + ) + + def has_coords(self) -> bool: + return self.gps_latitude is not None and self.gps_longitude is not None + def to_preview_sections(self) -> list[tuple[str, list[tuple[str, str]]]]: """Grouped preview: original capture, scan rig, process.""" sections: list[tuple[str, list[tuple[str, str]]]] = [] capture: list[tuple[str, str]] = [] + if self.capture_date is not None: + capture.append(("Date", self.capture_date.xmp_text())) if self.camera_make: capture.append(("Camera make", self.camera_make)) if self.camera_model: @@ -143,6 +167,13 @@ def to_preview_sections(self) -> list[tuple[str, list[tuple[str, str]]]]: capture.append(("Film format", self.film_format)) if self.film_color_type: capture.append(("Film type", self.film_color_type)) + place = self.place_display() + if place: + capture.append(("Place", place)) + if self.gps_latitude is not None and self.gps_longitude is not None: + coords = format_coords(self.gps_latitude, self.gps_longitude) + if coords != place: + capture.append(("Coordinates", coords)) if capture: sections.append(("Original capture", capture)) @@ -165,6 +196,10 @@ def to_preview_sections(self) -> list[tuple[str, list[tuple[str, str]]]]: scan.append(("ISO", str(self.scan_iso))) if self.scan_method: scan.append(("Scan method", self.scan_method)) + if self.scan_datetime: + scan.append(("Scan date", self.scan_datetime)) + if self.scan_gps_latitude is not None and self.scan_gps_longitude is not None: + scan.append(("Scan place", format_coords(self.scan_gps_latitude, self.scan_gps_longitude))) if self.capture_roll: scan.append(("Roll", self.capture_roll)) if self.capture_frame is not None: @@ -294,6 +329,7 @@ def build_metadata_payload( scan: ScanExif = extract_scan_from_exif(source_exif) push_pull = PUSH_PULL_LABELS.get(config.push_pull, "Normal") capture_exposure = config.exposure_override.strip() + capture_date = parse_capture_date(config.capture_date) draft = MetadataPayload( camera_make=camera_make.strip(), @@ -310,6 +346,12 @@ def build_metadata_payload( capture_exposure=capture_exposure, capture_roll=config.capture_roll.strip(), capture_frame=config.capture_frame, + capture_date=capture_date, + gps_latitude=config.gps_latitude, + gps_longitude=config.gps_longitude, + location_city=config.location_city.strip(), + location_state=config.location_state.strip(), + location_country=config.location_country.strip(), scan_camera_make=scan.camera_make, scan_camera_model=scan.camera_model, scan_lens_make=scan.lens_make, @@ -319,6 +361,9 @@ def build_metadata_payload( scan_iso=scan.iso, scan_exposure=scan.exposure, scan_method=config.scanning.strip(), + scan_datetime=scan.datetime_original, + scan_gps_latitude=scan.gps_latitude, + scan_gps_longitude=scan.gps_longitude, developer=config.developer.strip(), push_pull=push_pull, ) @@ -345,6 +390,12 @@ def build_metadata_payload( capture_exposure=draft.capture_exposure, capture_roll=draft.capture_roll, capture_frame=draft.capture_frame, + capture_date=draft.capture_date, + gps_latitude=draft.gps_latitude, + gps_longitude=draft.gps_longitude, + location_city=draft.location_city, + location_state=draft.location_state, + location_country=draft.location_country, scan_camera_make=draft.scan_camera_make, scan_camera_model=draft.scan_camera_model, scan_lens_make=draft.scan_lens_make, @@ -354,6 +405,9 @@ def build_metadata_payload( scan_iso=draft.scan_iso, scan_exposure=draft.scan_exposure, scan_method=draft.scan_method, + scan_datetime=draft.scan_datetime, + scan_gps_latitude=draft.scan_gps_latitude, + scan_gps_longitude=draft.scan_gps_longitude, image_description=desc, developer=draft.developer, push_pull=draft.push_pull, diff --git a/negpy/features/metadata/writer.py b/negpy/features/metadata/writer.py index 4dc64a0f..4a9408c3 100644 --- a/negpy/features/metadata/writer.py +++ b/negpy/features/metadata/writer.py @@ -12,7 +12,8 @@ import tifffile from PIL import Image, PngImagePlugin -from negpy.features.metadata.exif_read import strip_scan_exif_for_capture +from negpy.features.metadata.capture import exif_gps_rationals +from negpy.features.metadata.exif_read import strip_scan_exif_for_capture, strip_scan_gps from negpy.features.metadata.gear_models import GearLibrary from negpy.features.metadata.models import MetadataConfig from negpy.features.metadata.payload import NEGPY_SOFTWARE, MetadataPayload, build_metadata_payload @@ -131,7 +132,24 @@ def _build_custom_exif(payload: MetadataPayload) -> dict: if flags.exposure and payload.capture_exposure: exif.update(_parse_exposure_str(payload.capture_exposure)) - return {"0th": zeroth, "Exif": exif, "GPS": {}, "Interop": {}, "1st": {}} + if payload.capture_date is not None: + exif[piexif.ExifIFD.DateTimeOriginal] = _exif_ascii(payload.capture_date.exif_text()) + if payload.capture_date.tz_offset: + exif[piexif.ExifIFD.OffsetTimeOriginal] = _exif_ascii(payload.capture_date.tz_offset) + + gps: dict = {} + if payload.gps_latitude is not None and payload.gps_longitude is not None: + gps = exif_gps_rationals(payload.gps_latitude, payload.gps_longitude) + + return {"0th": zeroth, "Exif": exif, "GPS": gps, "Interop": {}, "1st": {}} + + +def _demote_scan_datetime(merged: dict) -> None: + """The source timestamp records when the frame was digitized, not when it was shot.""" + exif = merged.setdefault("Exif", {}) + source = exif.get(piexif.ExifIFD.DateTimeOriginal) + if source and piexif.ExifIFD.DateTimeDigitized not in exif: + exif[piexif.ExifIFD.DateTimeDigitized] = source def _sanitize_exif(exif_dict: dict) -> dict: @@ -340,6 +358,12 @@ def embed_metadata( if payload.exif_flags.strip_scan_residuals: strip_scan_exif_for_capture(merged) + if payload.capture_date is not None: + _demote_scan_datetime(merged) + + if payload.gps_latitude is not None and payload.gps_longitude is not None: + strip_scan_gps(merged) + custom = _build_custom_exif(payload) for ifd_name in ("0th", "Exif", "GPS", "Interop", "1st"): if ifd_name in custom and custom[ifd_name]: @@ -488,7 +512,7 @@ def _exif_bytes_to_extratags(exif_bytes: bytes) -> tuple[str | None, list[tuple] description = _decode_ascii(exif_dict.get("0th", {}).get(piexif.ImageIFD.ImageDescription)) extratags: list[tuple] = [] - for ifd_name in ("0th", "Exif", "GPS"): + for ifd_name in ("0th", "Exif"): ifd_data = exif_dict.get(ifd_name) or {} type_table = piexif.TAGS.get(ifd_name, {}) for tag, value in ifd_data.items(): diff --git a/negpy/features/metadata/xmp.py b/negpy/features/metadata/xmp.py index 51766f40..533e2ea1 100644 --- a/negpy/features/metadata/xmp.py +++ b/negpy/features/metadata/xmp.py @@ -4,6 +4,7 @@ import xml.etree.ElementTree as ET +from negpy.features.metadata.capture import xmp_gps from negpy.features.metadata.payload import MetadataPayload _XMP_BEGIN = '' @@ -13,6 +14,8 @@ "x": "adobe:ns:meta/", "rdf": "http://www.w3.org/1999/02/22-rdf-syntax-ns#", "dc": "http://purl.org/dc/elements/1.1/", + "photoshop": "http://ns.adobe.com/photoshop/1.0/", + "exif": "http://ns.adobe.com/exif/1.0/", "negpy": "https://negpy.app/ns/1.0/", } @@ -36,7 +39,7 @@ def build_xmp_xml(payload: MetadataPayload, *, standalone: bool = True) -> str: desc = ET.SubElement(rdf, f"{{{_NS['rdf']}}}Description") desc.set(f"{{{_NS['rdf']}}}about", "") - for prefix in ("dc", "negpy"): + for prefix in ("dc", "photoshop", "exif", "negpy"): desc.set(f"xmlns:{prefix}", _NS[prefix]) # negpy namespace: the original film capture. A structured mirror, with standard EXIF @@ -55,6 +58,11 @@ def build_xmp_xml(payload: MetadataPayload, *, standalone: bool = True) -> str: _sub(desc, "negpy", "CaptureMaxAperture", _to_rational(payload.max_aperture)) if payload.capture_exposure: _sub(desc, "negpy", "CaptureExposure", payload.capture_exposure) + if payload.capture_date is not None: + # photoshop:DateCreated keeps the truncated form; the precision word says whether + # "1998" meant a year or the first of January. + _sub(desc, "photoshop", "DateCreated", payload.capture_date.xmp_text()) + _sub(desc, "negpy", "CaptureDatePrecision", payload.capture_date.precision) if payload.iso is not None: _sub(desc, "negpy", "CaptureFilmISO", str(payload.iso)) if payload.film_stock: @@ -71,6 +79,17 @@ def build_xmp_xml(payload: MetadataPayload, *, standalone: bool = True) -> str: _sub(desc, "negpy", "PushPull", payload.push_pull) if payload.notes: _sub(desc, "negpy", "Notes", payload.notes) + if payload.location_city: + _sub(desc, "photoshop", "City", payload.location_city) + if payload.location_state: + _sub(desc, "photoshop", "State", payload.location_state) + if payload.location_country: + _sub(desc, "photoshop", "Country", payload.location_country) + if payload.gps_latitude is not None and payload.gps_longitude is not None: + lat, lon = xmp_gps(payload.gps_latitude, payload.gps_longitude) + _sub(desc, "exif", "GPSLatitude", lat) + _sub(desc, "exif", "GPSLongitude", lon) + _sub(desc, "exif", "GPSMapDatum", "WGS-84") if payload.scan_method: _sub(desc, "negpy", "ScanMethod", payload.scan_method) if payload.capture_roll: @@ -95,6 +114,8 @@ def build_xmp_xml(payload: MetadataPayload, *, standalone: bool = True) -> str: _sub(desc, "negpy", "ScanExposure", payload.scan_exposure) if payload.scan_iso is not None: _sub(desc, "negpy", "ScanISO", str(payload.scan_iso)) + if payload.scan_datetime: + _sub(desc, "negpy", "ScanDateTime", payload.scan_datetime) keywords: list[str] = [] for val in (payload.film_stock, payload.film_manufacturer, payload.film_format, payload.film_color_type): diff --git a/negpy/services/assets/search.py b/negpy/services/assets/search.py index 7237401e..9007a1ce 100644 --- a/negpy/services/assets/search.py +++ b/negpy/services/assets/search.py @@ -14,7 +14,10 @@ FLAG_FIELDS = frozenset({"keeper", "rejected", "edited"}) NUMERIC_FIELDS = frozenset({"iso", "frame", "push"}) -TEXT_FIELDS = frozenset({"name", "path", "ext", "film", "camera", "lens", "developer", "format", "scanning", "roll", "date"}) +# shot is truncated ISO-8601, so the prefix comparison below orders it without parsing a date. +TEXT_FIELDS = frozenset( + {"name", "path", "ext", "film", "camera", "lens", "developer", "format", "scanning", "roll", "date", "shot", "place"} +) FIELDS = FLAG_FIELDS | NUMERIC_FIELDS | TEXT_FIELDS _OPS = (">=", "<=", ">", "<") @@ -135,6 +138,8 @@ def facts_for(asset: dict, config: Any = None) -> dict[str, Any]: "frame": meta.capture_frame, "iso": meta.film_iso, "push": meta.push_pull, + "shot": meta.capture_date.casefold(), + "place": " ".join(p for p in (meta.location_city, meta.location_state, meta.location_country) if p).casefold(), } ) return facts diff --git a/negpy/services/export/templating.py b/negpy/services/export/templating.py index f0f45f7a..2caf5c59 100644 --- a/negpy/services/export/templating.py +++ b/negpy/services/export/templating.py @@ -6,6 +6,7 @@ from jinja2.sandbox import SandboxedEnvironment from negpy.domain.models import ExportConfig, ExportPreset, ExportResolutionMode +from negpy.features.metadata.capture import parse_capture_date from negpy.features.metadata.models import MetadataConfig from negpy.kernel.system.logging import get_logger @@ -74,6 +75,7 @@ def _metadata_context(original_stem: str, metadata: Optional[MetadataConfig]) -> camera = f"{meta.camera_make} {meta.camera_model}".strip() lens = meta.lens_model.strip() or meta.lens_make.strip() + captured = parse_capture_date(meta.capture_date) return { "roll": _path_safe(roll), @@ -95,6 +97,8 @@ def _metadata_context(original_stem: str, metadata: Optional[MetadataConfig]) -> "push_pull": meta.push_pull, "scanning": _path_safe(meta.scanning), "exposure": _path_safe(meta.exposure_override), + "capture_date": captured.compact() if captured else "", + "capture_year": str(captured.year) if captured else "", } @@ -124,6 +128,7 @@ def render_export_filename( - target_px: Target long edge in pixels (TARGET_PX mode only, else empty) - border: "border" if border size > 0, else empty - date: Current date in YYYYMMDD format + - capture_date / capture_year: Original capture date (YYYYMMDD / YYYY), empty when unset - roll / frame / frame_padded: Scanlight capture roll and frame (metadata or stem parse) - camera / camera_make / camera_model, lens / lens_make / lens_model / focal_length - film / film_iso / film_manufacturer / film_color_type / film_format diff --git a/negpy/services/maps.py b/negpy/services/maps.py new file mode 100644 index 00000000..8e09aa8c --- /dev/null +++ b/negpy/services/maps.py @@ -0,0 +1,126 @@ +"""OpenStreetMap tile and Nominatim place lookups for the capture-location picker.""" + +from __future__ import annotations + +import json +import os +import urllib.parse +import urllib.request +from typing import Any, Optional + +from negpy.kernel.system.config import APP_CONFIG +from negpy.kernel.system.logging import get_logger +from negpy.kernel.system.version import get_app_version + +logger = get_logger("services.maps") + +TILE_SIZE = 256 +MIN_ZOOM = 2 +MAX_ZOOM = 18 + +_TILE_URL = "https://tile.openstreetmap.org/{z}/{x}/{y}.png" +_NOMINATIM = "https://nominatim.openstreetmap.org" + +# Closing the picker joins whatever request is in flight, so this is also the longest stall a +# user can feel on OK or Cancel. +_TIMEOUT = 3.0 + +# The OSM tile policy requires an identifying User-Agent and local caching. +_USER_AGENT = f"NegPy/{get_app_version()} (+https://github.com/marcinz606/NegPy)" + +_CITY_KEYS = ("city", "town", "village", "hamlet", "municipality", "suburb") +_STATE_KEYS = ("state", "region", "province", "county") + + +def tile_cache_path(z: int, x: int, y: int) -> str: + return os.path.join(APP_CONFIG.cache_dir, "map_tiles", str(z), str(x), f"{y}.png") + + +def _get(url: str, timeout: float) -> Optional[bytes]: + try: + request = urllib.request.Request(url, headers={"User-Agent": _USER_AGENT}) + with urllib.request.urlopen(request, timeout=timeout) as response: + if response.status != 200: + return None + return response.read() + except Exception as exc: + logger.debug("map request failed (%s): %s", url, exc) + return None + + +def fetch_tile(z: int, x: int, y: int, timeout: float = _TIMEOUT) -> Optional[bytes]: + """One map tile, from the disk cache when it is there, otherwise from OSM.""" + path = tile_cache_path(z, x, y) + try: + with open(path, "rb") as fh: + return fh.read() + except OSError: + pass + + data = _get(_TILE_URL.format(z=z, x=x, y=y), timeout) + if data is None: + return None + + try: + os.makedirs(os.path.dirname(path), exist_ok=True) + tmp = f"{path}.tmp" + with open(tmp, "wb") as fh: + fh.write(data) + os.replace(tmp, path) + except OSError as exc: + logger.debug("map tile cache write failed: %s", exc) + + return data + + +def _json(url: str, timeout: float) -> Any: + data = _get(url, timeout) + if data is None: + return None + try: + return json.loads(data.decode("utf-8")) + except (UnicodeDecodeError, json.JSONDecodeError): + return None + + +def search_places(query: str, limit: int = 8, timeout: float = _TIMEOUT) -> list[dict]: + """Nominatim hits for a place name. Empty when offline or nothing matches.""" + if not query.strip(): + return [] + params = urllib.parse.urlencode({"q": query.strip(), "format": "json", "addressdetails": 1, "limit": limit}) + payload = _json(f"{_NOMINATIM}/search?{params}", timeout) + return [item for item in payload if isinstance(item, dict)] if isinstance(payload, list) else [] + + +def reverse_place(lat: float, lon: float, timeout: float = _TIMEOUT) -> Optional[dict]: + """The Nominatim result for a position, or None when it cannot be reached.""" + params = urllib.parse.urlencode({"lat": f"{lat:.6f}", "lon": f"{lon:.6f}", "format": "json", "addressdetails": 1, "zoom": 10}) + payload = _json(f"{_NOMINATIM}/reverse?{params}", timeout) + return payload if isinstance(payload, dict) and "error" not in payload else None + + +def place_fields(result: Optional[dict]) -> tuple[str, str, str]: + """City, state and country from a Nominatim result.""" + if not isinstance(result, dict): + return "", "", "" + address = result.get("address") + if not isinstance(address, dict): + return "", "", "" + + def first(keys: tuple[str, ...]) -> str: + for key in keys: + value = address.get(key) + if isinstance(value, str) and value.strip(): + return value.strip() + return "" + + return first(_CITY_KEYS), first(_STATE_KEYS), first(("country",)) + + +def result_coords(result: Optional[dict]) -> Optional[tuple[float, float]]: + if not isinstance(result, dict): + return None + try: + return float(result["lat"]), float(result["lon"]) + except (KeyError, TypeError, ValueError): + return None diff --git a/tests/metadata/test_capture.py b/tests/metadata/test_capture.py new file mode 100644 index 00000000..6a2ba309 --- /dev/null +++ b/tests/metadata/test_capture.py @@ -0,0 +1,151 @@ +"""Tests for capture date/place parsing and conversion.""" + +import piexif +import pytest + +from negpy.features.metadata.exif_read import gps_decimal +from negpy.features.metadata.capture import ( + deg2tile, + exif_gps_rationals, + format_coords, + parse_capture_date, + parse_coords, + place_summary, + tile2deg, + xmp_gps, +) + + +class TestParseCaptureDate: + @pytest.mark.parametrize( + "text, normalized, precision", + [ + ("1998", "1998", "year"), + ("1998-07", "1998-07", "month"), + ("1998-7", "1998-07", "month"), + ("1998/07/14", "1998-07-14", "day"), + ("1998-07-14 16:30", "1998-07-14 16:30", "minute"), + ("1998-07-14T16:30:05", "1998-07-14 16:30:05", "second"), + (" 2024-02-29 ", "2024-02-29", "day"), + ], + ) + def test_accepts_partial_forms(self, text: str, normalized: str, precision: str) -> None: + parsed = parse_capture_date(text) + assert parsed is not None + assert (parsed.text, parsed.precision) == (normalized, precision) + + @pytest.mark.parametrize( + "text", + ["", " ", "98", "1998-13", "1998-02-30", "2026-02-29", "1998-07-14 25:00", "yesterday", "1700", "3200"], + ) + def test_rejects_impossible_instants(self, text: str) -> None: + assert parse_capture_date(text) is None + + def test_offset_needs_a_time(self) -> None: + assert parse_capture_date("1998-07-14 16:30+02:00").tz_offset == "+02:00" + assert parse_capture_date("1998-07-14T16:30Z").tz_offset == "+00:00" + assert parse_capture_date("1998-07-14").tz_offset == "" + + def test_exif_pads_the_unknown_parts(self) -> None: + assert parse_capture_date("1998").exif_text() == "1998:01:01 00:00:00" + assert parse_capture_date("1998-07").exif_text() == "1998:07:01 00:00:00" + assert parse_capture_date("1998-07-14 16:30").exif_text() == "1998:07:14 16:30:00" + + def test_xmp_keeps_the_truncation(self) -> None: + assert parse_capture_date("1998-07").xmp_text() == "1998-07" + assert parse_capture_date("1998-07-14 16:30+02:00").xmp_text() == "1998-07-14 16:30+02:00" + + def test_compact_and_year_for_filenames(self) -> None: + parsed = parse_capture_date("1998-07") + assert (parsed.compact(), parsed.year) == ("19980701", 1998) + + +class TestParseCoords: + @pytest.mark.parametrize( + "text", + [ + "35.6762, 139.6503", + "35.6762 139.6503", + "https://www.openstreetmap.org/#map=13/35.6762/139.6503", + "https://www.openstreetmap.org/?mlat=35.6762&mlon=139.6503#map=16/35.6762/139.6503", + "https://www.google.com/maps/@35.6762,139.6503,15z", + "https://maps.google.com/?q=35.6762,139.6503", + ], + ) + def test_accepts_pairs_and_map_links(self, text: str) -> None: + lat, lon = parse_coords(text) + assert lat == pytest.approx(35.6762) + assert lon == pytest.approx(139.6503) + + @pytest.mark.parametrize("text", ["", "Tokyo", "91.0, 139.0", "35.0, 200.0", "35.0"]) + def test_rejects_non_positions(self, text: str) -> None: + assert parse_coords(text) is None + + def test_negative_pair(self) -> None: + assert parse_coords("-33.8688, -151.2093") == (-33.8688, -151.2093) + + +class TestPlaceSummary: + def test_names_win_over_coordinates(self) -> None: + assert place_summary("Tokyo", "", "Japan", 35.0, 139.0) == "Tokyo, Japan" + + def test_coordinates_when_no_names(self) -> None: + assert place_summary("", "", "", 35.0, 139.0) == format_coords(35.0, 139.0) + + def test_empty_when_nothing_is_set(self) -> None: + assert place_summary("", "", "", None, None) == "" + + +class TestGps: + def test_exif_rationals_and_hemispheres(self) -> None: + gps = exif_gps_rationals(-33.8688, 151.2093) + assert gps[piexif.GPSIFD.GPSLatitudeRef] == b"S" + assert gps[piexif.GPSIFD.GPSLongitudeRef] == b"E" + assert gps[piexif.GPSIFD.GPSMapDatum] == b"WGS-84" + degrees, minutes, seconds = gps[piexif.GPSIFD.GPSLatitude] + decimal = degrees[0] + minutes[0] / 60.0 + (seconds[0] / seconds[1]) / 3600.0 + assert decimal == pytest.approx(33.8688, abs=1e-4) + + def test_xmp_form(self) -> None: + lat, lon = xmp_gps(35.6762, -139.6503) + assert lat.startswith("35,40.57") and lat.endswith("N") + assert lon.startswith("139,39.01") and lon.endswith("W") + + +class TestTileMath: + @pytest.mark.parametrize("zoom", [2, 8, 18]) + @pytest.mark.parametrize("lat, lon", [(0.0, 0.0), (35.6762, 139.6503), (-33.8688, 151.2093)]) + def test_round_trip(self, lat: float, lon: float, zoom: int) -> None: + back_lat, back_lon = tile2deg(*deg2tile(lat, lon, zoom), zoom) + assert back_lat == pytest.approx(lat, abs=1e-6) + assert back_lon == pytest.approx(lon, abs=1e-6) + + def test_zoom_zero_centre_is_the_tile_middle(self) -> None: + assert deg2tile(0.0, 0.0, 1) == (1.0, 1.0) + + def test_latitude_clamps_at_the_mercator_limit(self) -> None: + _x, y = deg2tile(89.9, 0.0, 4) + assert 0.0 <= y <= 16.0 + + +class TestGpsDecimal: + @pytest.mark.parametrize( + "dms, ref, expected", + [ + (((51, 1), (30, 1), (0, 100)), b"N", 51.5), + (((0, 1), (7, 1), (3900, 100)), b"W", -0.1275), + (((33, 1), (52, 1), (768, 100)), b"S", -33.8688), + ], + ) + def test_hemisphere_signs_the_value(self, dms, ref, expected: float) -> None: + assert gps_decimal(dms, ref) == pytest.approx(expected, abs=1e-6) + + @pytest.mark.parametrize("dms", [None, (), ((1, 1), (2, 1)), ((1, 0), (2, 1), (3, 1)), "50"]) + def test_malformed_triplets_are_none(self, dms) -> None: + assert gps_decimal(dms, b"N") is None + + def test_round_trips_the_exif_writer(self) -> None: + rationals = exif_gps_rationals(35.6762, -139.6503) + lat = gps_decimal(rationals[piexif.GPSIFD.GPSLatitude], rationals[piexif.GPSIFD.GPSLatitudeRef]) + lon = gps_decimal(rationals[piexif.GPSIFD.GPSLongitude], rationals[piexif.GPSIFD.GPSLongitudeRef]) + assert (lat, lon) == pytest.approx((35.6762, -139.6503), abs=1e-4) diff --git a/tests/metadata/test_writer.py b/tests/metadata/test_writer.py index 372335b1..f8d7f603 100644 --- a/tests/metadata/test_writer.py +++ b/tests/metadata/test_writer.py @@ -307,3 +307,90 @@ def test_user_comment_fold_with_non_ascii_bytes(self) -> None: with tifffile.TiffFile(io.BytesIO(out)) as tf: desc = tf.pages[0].tags.get("ImageDescription") desc.value.encode("ascii") + + +def _jpeg_bytes() -> bytes: + from PIL import Image + + buf = io.BytesIO() + Image.new("RGB", (16, 16), (10, 20, 30)).save(buf, "JPEG") + return buf.getvalue() + + +def _source_exif(exif: dict | None = None) -> dict: + return {"0th": {}, "Exif": dict(exif or {}), "GPS": {}, "Interop": {}, "1st": {}} + + +class TestCaptureDateAndPlace: + def test_capture_date_replaces_the_scan_timestamp(self) -> None: + source = _source_exif({piexif.ExifIFD.DateTimeOriginal: b"2026:07:03 18:51:59"}) + out = embed_metadata(_jpeg_bytes(), MetadataConfig(capture_date="1998-07-14 16:30+02:00"), source) + exif = piexif.load(out)["Exif"] + assert exif[piexif.ExifIFD.DateTimeOriginal] == b"1998:07:14 16:30:00" + assert exif[piexif.ExifIFD.OffsetTimeOriginal] == b"+02:00" + # The scan is what was digitized, so its timestamp moves to that tag. + assert exif[piexif.ExifIFD.DateTimeDigitized] == b"2026:07:03 18:51:59" + + def test_partial_date_is_padded_for_exif_and_kept_in_xmp(self) -> None: + out = embed_metadata(_jpeg_bytes(), MetadataConfig(capture_date="1998"), _source_exif()) + assert piexif.load(out)["Exif"][piexif.ExifIFD.DateTimeOriginal] == b"1998:01:01 00:00:00" + assert b"1998" in out + assert b"year" in out + + def test_unset_capture_date_leaves_the_source_timestamp_alone(self) -> None: + source = _source_exif({piexif.ExifIFD.DateTimeOriginal: b"2026:07:03 18:51:59"}) + out = embed_metadata(_jpeg_bytes(), MetadataConfig(), source) + exif = piexif.load(out)["Exif"] + assert exif[piexif.ExifIFD.DateTimeOriginal] == b"2026:07:03 18:51:59" + assert piexif.ExifIFD.DateTimeDigitized not in exif + + def test_gps_ifd_and_place_names(self) -> None: + config = MetadataConfig( + gps_latitude=-33.8688, + gps_longitude=151.2093, + location_city="Sydney", + location_country="Australia", + ) + out = embed_metadata(_jpeg_bytes(), config, _source_exif()) + gps = piexif.load(out)["GPS"] + assert gps[piexif.GPSIFD.GPSLatitudeRef] == b"S" + assert gps[piexif.GPSIFD.GPSLongitudeRef] == b"E" + assert gps[piexif.GPSIFD.GPSMapDatum] == b"WGS-84" + assert b"Sydney" in out + assert b"" in out + + def test_tiff_carries_location_in_xmp_and_not_as_top_level_tags(self) -> None: + """GPS tag numbers 1-4 are not TIFF tags; a TIFF gets its location from XMP.""" + config = MetadataConfig(gps_latitude=35.6762, gps_longitude=139.6503) + out = embed_metadata(_make_tiff_bytes(), config, _source_exif()) + with tifffile.TiffFile(io.BytesIO(out)) as tf: + codes = {tag.code for tag in tf.pages[0].tags} + assert not codes & {1, 2, 3, 4} + assert b"" in out + + +class TestSourceGps: + _SOURCE_GPS = { + piexif.GPSIFD.GPSLatitudeRef: b"N", + piexif.GPSIFD.GPSLatitude: ((51, 1), (30, 1), (0, 100)), + piexif.GPSIFD.GPSLongitudeRef: b"W", + piexif.GPSIFD.GPSLongitude: ((0, 1), (7, 1), (3900, 100)), + piexif.GPSIFD.GPSAltitude: (35, 1), + } + + def _with_gps(self) -> dict: + source = _source_exif() + source["GPS"] = dict(self._SOURCE_GPS) + return source + + def test_source_position_survives_when_no_place_is_set(self) -> None: + out = embed_metadata(_jpeg_bytes(), MetadataConfig(), self._with_gps()) + assert piexif.load(out)["GPS"] == self._SOURCE_GPS + + def test_picked_place_replaces_the_whole_source_block(self) -> None: + """Keeping the scan's altitude or heading beside our coordinates would mix two places.""" + config = MetadataConfig(gps_latitude=35.6762, gps_longitude=139.6503) + out = embed_metadata(_jpeg_bytes(), config, self._with_gps()) + gps = piexif.load(out)["GPS"] + assert piexif.GPSIFD.GPSAltitude not in gps + assert gps[piexif.GPSIFD.GPSLongitudeRef] == b"E" diff --git a/tests/test_asset_search.py b/tests/test_asset_search.py index 65ad53a0..a9467ffb 100644 --- a/tests/test_asset_search.py +++ b/tests/test_asset_search.py @@ -26,6 +26,8 @@ def _facts(**overrides): "frame": 7, "iso": 400, "push": 0, + "shot": "", + "place": "", } base.update(overrides) return base @@ -177,3 +179,56 @@ def test_facts_for_edited_asset_reads_metadata_and_roll(): def test_facts_for_bad_mtime_yields_empty_date(): assert facts_for({"name": "a.nef", "mtime": None}, None)["date"] == "" assert _hits("date:2024", date="") is False + + +@pytest.mark.parametrize( + "query, expected", + [ + ("shot:1998", True), + ("shot:1998-07", True), + ("shot:1999", False), + ("shot:>=1998-07", True), + ("shot:>=1998-08", False), + ("shot:<2000", True), + ("shot:>1998-07-14", False), + ("-shot:1998", False), + ], +) +def test_capture_date_orders_without_parsing_a_date(query: str, expected: bool): + """`shot` is truncated ISO-8601, so a prefix comparison is the whole ordering.""" + assert _hits(query, shot="1998-07-14 16:30") is expected + + +def test_capture_date_is_not_the_file_date(): + assert _hits("shot:2024", shot="1998") is False + assert _hits("date:2024") is True + + +def test_a_year_only_capture_date_is_not_claimed_to_be_in_any_month(): + assert _hits("shot:1998", shot="1998") is True + assert _hits("shot:>=1998-07", shot="1998") is False + + +@pytest.mark.parametrize("query, expected", [("place:tokyo", True), ("place:japan", True), ("place:paris", False)]) +def test_place_matches_any_of_city_state_country(query: str, expected: bool): + assert _hits(query, place="tokyo tokyo japan") is expected + + +def test_unset_capture_date_and_place_never_match(): + assert _hits("shot:1998") is False + assert _hits("place:tokyo") is False + + +def test_facts_for_edited_asset_reads_capture_date_and_place(): + config = replace( + WorkspaceConfig(), + metadata=MetadataConfig( + capture_date="1998-07-14 16:30", + location_city="Tokyo", + location_state="Tokyo", + location_country="Japan", + ), + ) + facts = facts_for({"name": "a.nef", "mtime": 0.0}, config) + assert facts["shot"] == "1998-07-14 16:30" + assert facts["place"] == "tokyo tokyo japan" diff --git a/tests/test_config_deserialization.py b/tests/test_config_deserialization.py index 878dbcb5..2beeceee 100644 --- a/tests/test_config_deserialization.py +++ b/tests/test_config_deserialization.py @@ -359,6 +359,26 @@ def test_no_sub_config_is_missing_from_the_known_keys_set(self): with self.assertNoLogs("negpy.domain.models", level=logging.WARNING): WorkspaceConfig.from_flat_dict(WorkspaceConfig().to_dict()) + def test_capture_date_and_place_survive_json(self): + """The place is Optional[float] plus strings, so a null must not become 0.0.""" + config = replace( + WorkspaceConfig(), + metadata=replace( + WorkspaceConfig().metadata, + capture_date="1998-07", + gps_latitude=35.6762, + gps_longitude=139.6503, + location_city="Tokyo", + ), + ) + loaded = WorkspaceConfig.from_flat_dict(json.loads(json.dumps(config.to_dict()))).metadata + self.assertEqual(loaded.capture_date, "1998-07") + self.assertEqual((loaded.gps_latitude, loaded.gps_longitude), (35.6762, 139.6503)) + self.assertEqual(loaded.location_city, "Tokyo") + + unset = WorkspaceConfig.from_flat_dict(json.loads(json.dumps(WorkspaceConfig().to_dict()))) + self.assertIsNone(unset.metadata.gps_latitude) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_location_picker_dialog.py b/tests/test_location_picker_dialog.py new file mode 100644 index 00000000..af455e3e --- /dev/null +++ b/tests/test_location_picker_dialog.py @@ -0,0 +1,108 @@ +"""Offline tests for the capture-location picker. Every network call is patched.""" + +import os +import sys + +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") + +from PyQt6.QtWidgets import QApplication # noqa: E402 + +from negpy.desktop.view.widgets.location_picker_dialog import LocationPickerDialog # noqa: E402 + +if not QApplication.instance(): + _app = QApplication(sys.argv) + + +_TOKYO = { + "display_name": "Tokyo, Japan", + "lat": "35.6762", + "lon": "139.6503", + "address": {"city": "Tokyo", "state": "Tokyo", "country": "Japan"}, +} + + +def _dialog(monkeypatch, **kwargs) -> LocationPickerDialog: + """Lookups run inline: no thread may outlive the test and reach the network.""" + monkeypatch.setattr("negpy.services.maps.reverse_place", lambda *a, **k: None) + monkeypatch.setattr("negpy.desktop.view.widgets.slippy_map.fetch_tile", lambda *a, **k: None) + monkeypatch.setattr( + "negpy.desktop.view.widgets.location_picker_dialog.reverse_place", + lambda *a, **k: None, + ) + dialog = LocationPickerDialog(**kwargs) + monkeypatch.setattr(dialog._pool, "start", lambda job, *args: job.run()) + return dialog + + +def test_opens_with_the_existing_location(monkeypatch) -> None: + dlg = _dialog(monkeypatch, lat=35.6586, lon=139.7454, city="Tokyo", country="Japan") + assert dlg.location() == (35.6586, 139.7454, "Tokyo", "", "Japan") + assert dlg.map_view.pin() == (35.6586, 139.7454) + + +def test_search_result_sets_pin_and_place(monkeypatch) -> None: + dlg = _dialog(monkeypatch) + dlg._on_search_done([_TOKYO]) + assert dlg.results_list.isVisible() is False or dlg.results_list.count() == 1 + dlg._on_result_selected(0) + lat, lon, city, state, country = dlg.location() + assert (round(lat, 4), round(lon, 4)) == (35.6762, 139.6503) + assert (city, state, country) == ("Tokyo", "Tokyo", "Japan") + + +def test_empty_search_result_reports_unavailable(monkeypatch) -> None: + dlg = _dialog(monkeypatch) + dlg._on_search_done([]) + assert "unavailable" in dlg.status_label.text() + + +def test_pasted_map_link_moves_the_pin(monkeypatch) -> None: + dlg = _dialog(monkeypatch) + dlg.coords_edit.setText("https://www.openstreetmap.org/#map=13/49.5/19.5") + dlg._on_coords_edited() + assert dlg.map_view.pin() == (49.5, 19.5) + assert dlg.location()[:2] == (49.5, 19.5) + + +def test_unparsable_coordinates_are_reported_and_not_applied(monkeypatch) -> None: + dlg = _dialog(monkeypatch) + dlg.coords_edit.setText("somewhere nice") + dlg._on_coords_edited() + assert dlg.location()[:2] == (None, None) + assert "not recognised" in dlg.status_label.text() + + +def test_clicking_the_map_fills_place_from_reverse_lookup(monkeypatch) -> None: + dlg = _dialog(monkeypatch) + dlg._on_pin_moved(35.6762, 139.6503) + dlg._on_reverse_done(dlg._reverse_token, _TOKYO) + assert dlg.location()[2:] == ("Tokyo", "Tokyo", "Japan") + + +def test_stale_reverse_lookup_is_ignored(monkeypatch) -> None: + dlg = _dialog(monkeypatch, city="Kyoto") + dlg._on_pin_moved(35.6762, 139.6503) + dlg._on_reverse_done(dlg._reverse_token - 1, _TOKYO) + assert dlg.location()[2] == "Kyoto" + + +def test_reverse_failure_keeps_the_coordinates(monkeypatch) -> None: + dlg = _dialog(monkeypatch) + dlg._on_pin_moved(35.6762, 139.6503) + dlg._on_reverse_done(dlg._reverse_token, None) + assert dlg.location()[:2] == (35.6762, 139.6503) + assert "unavailable" in dlg.status_label.text() + + +def test_centre_frames_the_view_without_claiming_the_place(monkeypatch) -> None: + """A scan file's coordinates say where it was digitized, not where it was shot.""" + dlg = _dialog(monkeypatch, center=(35.6762, 139.6503)) + assert dlg.map_view.pin() is None + assert dlg.location() == (None, None, "", "", "") + assert dlg.map_view._center == (35.6762, 139.6503) + assert "scan file" in dlg.status_label.text() + + +def test_an_existing_place_wins_over_the_centre(monkeypatch) -> None: + dlg = _dialog(monkeypatch, lat=35.6586, lon=139.7454, center=(0.0, 0.0)) + assert dlg.map_view.pin() == (35.6586, 139.7454) diff --git a/tests/test_maps.py b/tests/test_maps.py new file mode 100644 index 00000000..00b4c153 --- /dev/null +++ b/tests/test_maps.py @@ -0,0 +1,114 @@ +"""Offline tests for the OpenStreetMap helpers: no request leaves the process.""" + +from __future__ import annotations + +import json + +import pytest + +from negpy.services import maps + + +class _Response: + def __init__(self, payload: bytes, status: int = 200): + self._payload = payload + self.status = status + + def read(self) -> bytes: + return self._payload + + def __enter__(self): + return self + + def __exit__(self, *_exc) -> None: + return None + + +def _patch_urlopen(monkeypatch, payload: object, status: int = 200) -> list[str]: + urls: list[str] = [] + + def fake(request, timeout=None): + urls.append(request.full_url) + assert request.headers["User-agent"].startswith("NegPy/") + if payload is None: + raise OSError("offline") + body = payload if isinstance(payload, bytes) else json.dumps(payload).encode() + return _Response(body, status) + + monkeypatch.setattr(maps.urllib.request, "urlopen", fake) + return urls + + +class TestSearchPlaces: + def test_returns_results(self, monkeypatch) -> None: + urls = _patch_urlopen(monkeypatch, [{"display_name": "Tokyo"}]) + assert maps.search_places("Tokyo") == [{"display_name": "Tokyo"}] + assert "addressdetails=1" in urls[0] + + def test_blank_query_makes_no_request(self, monkeypatch) -> None: + urls = _patch_urlopen(monkeypatch, []) + assert maps.search_places(" ") == [] + assert urls == [] + + def test_offline_is_empty(self, monkeypatch) -> None: + _patch_urlopen(monkeypatch, None) + assert maps.search_places("Tokyo") == [] + + def test_non_json_is_empty(self, monkeypatch) -> None: + _patch_urlopen(monkeypatch, b"rate limited") + assert maps.search_places("Tokyo") == [] + + +class TestReversePlace: + def test_returns_result(self, monkeypatch) -> None: + _patch_urlopen(monkeypatch, {"address": {"city": "Tokyo"}}) + assert maps.reverse_place(35.0, 139.0) == {"address": {"city": "Tokyo"}} + + def test_error_payload_is_none(self, monkeypatch) -> None: + _patch_urlopen(monkeypatch, {"error": "Unable to geocode"}) + assert maps.reverse_place(0.0, 0.0) is None + + def test_offline_is_none(self, monkeypatch) -> None: + _patch_urlopen(monkeypatch, None) + assert maps.reverse_place(35.0, 139.0) is None + + +class TestPlaceFields: + @pytest.mark.parametrize( + "address, city", + [ + ({"city": "Tokyo"}, "Tokyo"), + ({"village": "Ōgimi"}, "Ōgimi"), + ({"town": "Hakone", "city": "Tokyo"}, "Tokyo"), + ({}, ""), + ], + ) + def test_city_falls_back_through_the_settlement_keys(self, address: dict, city: str) -> None: + assert maps.place_fields({"address": address})[0] == city + + def test_missing_address_is_empty(self) -> None: + assert maps.place_fields({"lat": "35"}) == ("", "", "") + assert maps.place_fields(None) == ("", "", "") + + +class TestFetchTile: + def test_disk_cache_is_used_before_the_network(self, monkeypatch, tmp_path) -> None: + path = tmp_path / "9" / "1" / "2.png" + path.parent.mkdir(parents=True) + path.write_bytes(b"cached") + monkeypatch.setattr(maps, "tile_cache_path", lambda z, x, y: str(path)) + urls = _patch_urlopen(monkeypatch, b"downloaded") + assert maps.fetch_tile(9, 1, 2) == b"cached" + assert urls == [] + + def test_download_writes_the_cache(self, monkeypatch, tmp_path) -> None: + path = tmp_path / "9" / "1" / "2.png" + monkeypatch.setattr(maps, "tile_cache_path", lambda z, x, y: str(path)) + _patch_urlopen(monkeypatch, b"downloaded") + assert maps.fetch_tile(9, 1, 2) == b"downloaded" + assert path.read_bytes() == b"downloaded" + + def test_offline_returns_none(self, monkeypatch, tmp_path) -> None: + monkeypatch.setattr(maps, "tile_cache_path", lambda z, x, y: str(tmp_path / "t.png")) + _patch_urlopen(monkeypatch, None) + assert maps.fetch_tile(9, 1, 2) is None diff --git a/tests/test_metadata_sidebar.py b/tests/test_metadata_sidebar.py new file mode 100644 index 00000000..4284475b --- /dev/null +++ b/tests/test_metadata_sidebar.py @@ -0,0 +1,172 @@ +"""Offline tests for the Metadata panel's Capture card.""" + +from __future__ import annotations + +import os + +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") + +import sys + +from dataclasses import replace + +import piexif +import pytest +from PyQt6.QtWidgets import QApplication, QLabel + +from conftest import FakeController +from negpy.desktop.view.sidebar import metadata as metadata_module +from negpy.desktop.view.sidebar.metadata import MetadataSidebar +from negpy.features.metadata.gear_models import GearLibrary + +if not QApplication.instance(): + _app = QApplication(sys.argv) + + +@pytest.fixture +def sidebar(monkeypatch) -> MetadataSidebar: + monkeypatch.setattr(metadata_module.GearProfiles, "load_library", staticmethod(GearLibrary)) + controller = FakeController() + controller.session.update_config = lambda config, **_kwargs: setattr(controller.state, "config", config) + return MetadataSidebar(controller) + + +def _set_metadata(sidebar: MetadataSidebar, **changes) -> None: + state = sidebar.state + state.config = replace(state.config, metadata=replace(state.config.metadata, **changes)) + + +class TestCaptureDate: + def test_invalid_date_is_flagged_and_not_persisted(self, sidebar: MetadataSidebar) -> None: + _set_metadata(sidebar, capture_date="1998-07") + sidebar.capture_date_edit.setText("1998-13") + assert sidebar.capture_date_edit.styleSheet() != "" + sidebar._persist_all_metadata_settings() + assert sidebar.state.config.metadata.capture_date == "1998-07" + + def test_valid_date_is_normalized_on_persist(self, sidebar: MetadataSidebar) -> None: + sidebar.capture_date_edit.setText("1998/7/4 16:30") + assert sidebar.capture_date_edit.styleSheet() == "" + sidebar._persist_all_metadata_settings() + assert sidebar.state.config.metadata.capture_date == "1998-07-04 16:30" + + def test_cleared_date_persists_as_unset(self, sidebar: MetadataSidebar) -> None: + _set_metadata(sidebar, capture_date="1998") + sidebar.sync_ui() + sidebar.capture_date_edit.setText("") + sidebar._persist_all_metadata_settings() + assert sidebar.state.config.metadata.capture_date == "" + + +class TestCapturePlace: + def test_place_field_shows_names_then_coordinates(self, sidebar: MetadataSidebar) -> None: + _set_metadata(sidebar, location_city="Tokyo", location_country="Japan") + sidebar.sync_ui() + assert sidebar.place_edit.text() == "Tokyo, Japan" + + _set_metadata(sidebar, location_city="", location_country="", gps_latitude=35.0, gps_longitude=139.0) + sidebar.sync_ui() + assert sidebar.place_edit.text() == "35.00000, 139.00000" + + def test_pasted_map_link_sets_coordinates_and_keeps_names(self, sidebar: MetadataSidebar) -> None: + _set_metadata(sidebar, location_city="Tokyo") + sidebar.place_edit.setText("https://www.openstreetmap.org/#map=13/49.5/19.5") + sidebar._on_place_edited() + conf = sidebar.state.config.metadata + assert (conf.gps_latitude, conf.gps_longitude) == (49.5, 19.5) + assert conf.location_city == "Tokyo" + + def test_unparsable_place_text_is_reverted(self, sidebar: MetadataSidebar) -> None: + _set_metadata(sidebar, location_city="Tokyo", location_country="Japan") + sidebar.place_edit.setText("somewhere nice") + sidebar._on_place_edited() + assert sidebar.place_edit.text() == "Tokyo, Japan" + assert sidebar.state.config.metadata.gps_latitude is None + + def test_clear_empties_position_and_names(self, sidebar: MetadataSidebar) -> None: + _set_metadata(sidebar, location_city="Tokyo", gps_latitude=35.0, gps_longitude=139.0) + sidebar._on_place_clear() + conf = sidebar.state.config.metadata + assert (conf.gps_latitude, conf.gps_longitude, conf.location_city) == (None, None, "") + assert sidebar.place_edit.text() == "" + + def test_picker_result_is_applied(self, sidebar: MetadataSidebar, monkeypatch) -> None: + class _StubDialog: + DialogCode = metadata_module.LocationPickerDialog.DialogCode + + def __init__(self, *_args, **_kwargs): + pass + + def exec(self): + return self.DialogCode.Accepted + + def location(self): + return 35.6762, 139.6503, "Tokyo", "Tokyo", "Japan" + + monkeypatch.setattr(metadata_module, "LocationPickerDialog", _StubDialog) + sidebar._open_location_picker() + conf = sidebar.state.config.metadata + assert (conf.gps_latitude, conf.location_city, conf.location_country) == ( + 35.6762, + "Tokyo", + "Japan", + ) + assert sidebar.place_edit.text() == "Tokyo, Tokyo, Japan" + + +class TestSourceGpsPrefill: + _SCAN_GPS = { + "GPS": { + piexif.GPSIFD.GPSLatitude: ((35, 1), (40, 1), (3432, 100)), + piexif.GPSIFD.GPSLatitudeRef: b"N", + piexif.GPSIFD.GPSLongitude: ((139, 1), (39, 1), (108, 100)), + piexif.GPSIFD.GPSLongitudeRef: b"E", + } + } + + @staticmethod + def _capture_picker(monkeypatch) -> dict: + seen: dict = {} + + class _StubDialog: + DialogCode = metadata_module.LocationPickerDialog.DialogCode + + def __init__(self, *args, **kwargs): + seen.update(kwargs) + + def exec(self): + return self.DialogCode.Rejected + + monkeypatch.setattr(metadata_module, "LocationPickerDialog", _StubDialog) + return seen + + def _with_scan_exif(self, sidebar: MetadataSidebar) -> None: + sidebar.state.current_file_hash = "hash1" + sidebar.state.source_exif["hash1"] = self._SCAN_GPS + + def test_picker_opens_on_the_scan_position(self, sidebar: MetadataSidebar, monkeypatch) -> None: + self._with_scan_exif(sidebar) + seen = self._capture_picker(monkeypatch) + sidebar._open_location_picker() + assert seen["center"] == pytest.approx((35.6762, 139.6503), abs=1e-4) + + def test_scan_position_is_not_adopted_as_the_capture_place(self, sidebar: MetadataSidebar) -> None: + self._with_scan_exif(sidebar) + sidebar.sync_ui() + assert sidebar.place_edit.text() == "" + assert sidebar.state.config.metadata.gps_latitude is None + + def test_scan_position_shows_in_the_preview(self, sidebar: MetadataSidebar) -> None: + self._with_scan_exif(sidebar) + sidebar._update_preview() + labels = [sidebar.preview_rows.itemAt(i).widget().findChildren(QLabel) for i in range(sidebar.preview_rows.count())] + texts = [label.text() for row in labels for label in row] + assert "Scan place" in texts + assert "35.67620, 139.65030" in texts + + def test_an_existing_place_needs_no_centre(self, sidebar: MetadataSidebar, monkeypatch) -> None: + self._with_scan_exif(sidebar) + _set_metadata(sidebar, gps_latitude=1.0, gps_longitude=2.0) + seen = self._capture_picker(monkeypatch) + sidebar._open_location_picker() + assert seen["center"] is None diff --git a/tests/test_slippy_map.py b/tests/test_slippy_map.py new file mode 100644 index 00000000..48ce895c --- /dev/null +++ b/tests/test_slippy_map.py @@ -0,0 +1,157 @@ +"""Offline tests for the map widget: tiles are stubbed, nothing reaches the network.""" + +from __future__ import annotations + +import os + +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") + +import sys + +import pytest +from PyQt6.QtCore import QPoint, QPointF, Qt +from PyQt6.QtGui import QMouseEvent, QWheelEvent +from PyQt6.QtWidgets import QApplication + +from negpy.desktop.view.widgets import slippy_map +from negpy.desktop.view.widgets.slippy_map import SlippyMapWidget + +if not QApplication.instance(): + _app = QApplication(sys.argv) + + +@pytest.fixture +def widget(monkeypatch) -> SlippyMapWidget: + monkeypatch.setattr(slippy_map, "fetch_tile", lambda *a, **k: None) + map_widget = SlippyMapWidget() + map_widget.resize(512, 384) + monkeypatch.setattr(map_widget._pool, "start", lambda job, *args: job.run()) + return map_widget + + +def _click(widget: SlippyMapWidget, x: int, y: int) -> None: + for event_type in (QMouseEvent.Type.MouseButtonPress, QMouseEvent.Type.MouseButtonRelease): + widget.event( + QMouseEvent( + event_type, + QPointF(x, y), + Qt.MouseButton.LeftButton, + Qt.MouseButton.LeftButton, + Qt.KeyboardModifier.NoModifier, + ) + ) + + +def test_paints_with_no_tiles_available(widget: SlippyMapWidget) -> None: + """A missing tile must paint a placeholder, not raise.""" + widget.set_pin(50.0614, 19.9366) + assert not widget.grab().isNull() + + +def test_click_sets_the_pin_at_the_cursor(widget: SlippyMapWidget) -> None: + expected = widget.latlon_at(100, 80) + _click(widget, 100, 80) + assert widget.pin() == pytest.approx(expected) + + +def test_centre_of_the_widget_is_the_centre_of_the_view(widget: SlippyMapWidget) -> None: + widget.set_pin(50.0614, 19.9366) + x, y = widget._pixel_at(50.0614, 19.9366) + assert (x, y) == pytest.approx((widget.width() / 2.0, widget.height() / 2.0)) + + +def test_drag_pans_without_moving_the_pin(widget: SlippyMapWidget) -> None: + widget.set_pin(50.0, 19.0) + press = QMouseEvent( + QMouseEvent.Type.MouseButtonPress, + QPointF(200, 200), + Qt.MouseButton.LeftButton, + Qt.MouseButton.LeftButton, + Qt.KeyboardModifier.NoModifier, + ) + move = QMouseEvent( + QMouseEvent.Type.MouseMove, + QPointF(260, 200), + Qt.MouseButton.NoButton, + Qt.MouseButton.LeftButton, + Qt.KeyboardModifier.NoModifier, + ) + release = QMouseEvent( + QMouseEvent.Type.MouseButtonRelease, + QPointF(260, 200), + Qt.MouseButton.LeftButton, + Qt.MouseButton.LeftButton, + Qt.KeyboardModifier.NoModifier, + ) + for event in (press, move, release): + widget.event(event) + + assert widget.pin() == (50.0, 19.0) + # Dragging right shows what is west of the old centre. + assert widget._pixel_at(50.0, 19.0)[0] > widget.width() / 2.0 + + +def test_wheel_zoom_keeps_the_position_under_the_cursor(widget: SlippyMapWidget) -> None: + anchor = widget.latlon_at(120.0, 90.0) + widget.event( + QWheelEvent( + QPointF(120.0, 90.0), + QPointF(120.0, 90.0), + QPoint(0, 0), + QPoint(0, 120), + Qt.MouseButton.NoButton, + Qt.KeyboardModifier.NoModifier, + Qt.ScrollPhase.NoScrollPhase, + False, + ) + ) + assert widget._zoom == 5 + assert widget.latlon_at(120.0, 90.0) == pytest.approx(anchor, abs=1e-6) + + +def test_zoom_clamps_to_the_supported_range(widget: SlippyMapWidget) -> None: + widget.set_zoom(99) + assert widget._zoom == slippy_map.MAX_ZOOM + widget.set_zoom(0) + assert widget._zoom == slippy_map.MIN_ZOOM + + +def test_pending_tiles_are_capped(monkeypatch) -> None: + """The pool joins its queue on close, so the queue must stay small.""" + monkeypatch.setattr(slippy_map, "fetch_tile", lambda *a, **k: None) + map_widget = SlippyMapWidget() + map_widget.resize(512, 384) + started: list[tuple] = [] + monkeypatch.setattr(map_widget._pool, "start", lambda job, *args: started.append(job)) + + for x in range(200): + map_widget._request((4, x, 4)) + + assert len(started) == slippy_map._MAX_PENDING_TILES + + +def test_shutdown_stops_queued_fetches(monkeypatch) -> None: + fetched: list[tuple] = [] + monkeypatch.setattr(slippy_map, "fetch_tile", lambda *a, **k: fetched.append(a)) + map_widget = SlippyMapWidget() + queued: list = [] + monkeypatch.setattr(map_widget._pool, "start", lambda job, *args: queued.append(job)) + map_widget._request((4, 1, 1)) + + map_widget.shutdown() + for job in queued: + job.run() + + assert fetched == [] + + +def test_shutdown_joins_running_fetches_instead_of_the_destructor(monkeypatch) -> None: + """The pool's destructor waits with the GIL held, which would hang the GUI for good.""" + waited: list[int] = [] + monkeypatch.setattr(slippy_map, "fetch_tile", lambda *a, **k: None) + map_widget = SlippyMapWidget() + monkeypatch.setattr(map_widget._pool, "waitForDone", lambda ms: waited.append(ms) or True) + + map_widget.shutdown() + + assert waited == [slippy_map._SHUTDOWN_WAIT_MS] diff --git a/tests/test_templating.py b/tests/test_templating.py index a1087315..3c1a0def 100644 --- a/tests/test_templating.py +++ b/tests/test_templating.py @@ -297,3 +297,14 @@ def test_percent_format_with_frame_set(): meta = MetadataConfig(capture_roll="R1", capture_frame=12) conf = ExportConfig(filename_pattern='{{ roll }}_Frame{{ "%03d" % frame }}') assert render_export_filename("shot.tif", conf, metadata=meta) == "R1_Frame012" + + +def test_capture_date_vars(): + meta = MetadataConfig(capture_date="1998-07") + conf = ExportConfig(filename_pattern="{{ capture_year }}_{{ capture_date }}_{{ original_name }}") + assert render_export_filename("shot.tif", conf, metadata=meta) == "1998_19980701_shot" + + +def test_capture_date_vars_empty_when_unset(): + conf = ExportConfig(filename_pattern="{{ capture_year }}_{{ original_name }}") + assert render_export_filename("shot.tif", conf, metadata=MetadataConfig()) == "shot" From f051012d8818b4d8d2fd9d8781b7aa644fac58fe Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Wed, 19 Aug 2026 17:49:52 +0200 Subject: [PATCH 2/4] feat(metadata): suggest places in a dropdown as the user types The search hits filled a list under the field, which took layout space from the map and needed a button press to appear. They are now a QCompleter dropdown on the search field, filled from the geocoder and shown unfiltered, because filtering the server's hits again locally would hide "Tokio" for "tokyo". A 500 ms timer holds the request until typing pauses, and a query under three characters never searches, which keeps the app inside Nominatim's one-request-a-second policy. Enter still searches at once. The lookups also ask for the user's language now: without it Nominatim answers in the local script, and that name goes into XMP. --- .../view/widgets/location_picker_dialog.py | 87 ++++++++++++------- negpy/services/maps.py | 23 ++++- tests/test_location_picker_dialog.py | 67 +++++++++++++- tests/test_maps.py | 21 +++++ 4 files changed, 160 insertions(+), 38 deletions(-) diff --git a/negpy/desktop/view/widgets/location_picker_dialog.py b/negpy/desktop/view/widgets/location_picker_dialog.py index 3befc616..03a93d6f 100644 --- a/negpy/desktop/view/widgets/location_picker_dialog.py +++ b/negpy/desktop/view/widgets/location_picker_dialog.py @@ -4,15 +4,14 @@ from typing import Optional -from PyQt6.QtCore import QObject, QRunnable, QThreadPool, pyqtSignal +import qtawesome as qta +from PyQt6.QtCore import QModelIndex, QObject, QRunnable, QStringListModel, Qt, QThreadPool, QTimer, pyqtSignal from PyQt6.QtWidgets import ( + QCompleter, QDialog, QDialogButtonBox, QGridLayout, - QHBoxLayout, QLineEdit, - QListWidget, - QPushButton, QVBoxLayout, ) @@ -23,7 +22,11 @@ from negpy.services.maps import place_fields, result_coords, reverse_place, search_places _OFFLINE_HINT = "Map unavailable — enter coordinates manually." +_NO_MATCH_HINT = "No place matched, or the lookup is unreachable." _SHUTDOWN_WAIT_MS = 6000 +# Nominatim asks for at most one request a second, so a keystroke must not be a request. +_SEARCH_DEBOUNCE_MS = 500 +_MIN_QUERY_CHARS = 3 class _LookupSignals(QObject): @@ -86,7 +89,7 @@ def __init__( self._signals.search_done.connect(self._on_search_done) self._signals.reverse_done.connect(self._on_reverse_done) self._reverse_token = 0 - self._results: list[dict] = [] + self._results: dict[str, dict] = {} root = QVBoxLayout(self) root.setContentsMargins(THEME.space_xl, THEME.space_xl, THEME.space_xl, THEME.space_xl) @@ -96,22 +99,31 @@ def __init__( hint_label("Search a place, click the map, or paste coordinates or a map link. Opening this dialog contacts OpenStreetMap.") ) - search_row = QHBoxLayout() - search_row.setSpacing(THEME.space_sm) self.search_edit = QLineEdit() self.search_edit.setPlaceholderText("e.g. Tokyo, Japan") + self.search_edit.addAction( + qta.icon("fa5s.search", color=THEME.text_secondary), + QLineEdit.ActionPosition.LeadingPosition, + ) + self.search_edit.textEdited.connect(self._on_search_text_edited) self.search_edit.returnPressed.connect(self._on_search) - self.search_btn = QPushButton("Search") - self.search_btn.clicked.connect(self._on_search) - search_row.addWidget(self.search_edit, 1) - search_row.addWidget(self.search_btn) - root.addLayout(search_row) - - self.results_list = QListWidget() - self.results_list.setMaximumHeight(96) - self.results_list.setVisible(False) - self.results_list.currentRowChanged.connect(self._on_result_selected) - root.addWidget(self.results_list) + root.addWidget(self.search_edit) + + # A QCompleter, not a hand-rolled Qt.Popup: a popup grabs the keyboard, so the next + # keystroke would go to the list instead of the field. Unfiltered, because the hits + # come from the geocoder — filtering them again locally would hide "Tōkyō" for "tokyo". + self._suggestions = QStringListModel(self) + self._completer = QCompleter(self._suggestions, self) + self._completer.setCompletionMode(QCompleter.CompletionMode.UnfilteredPopupCompletion) + self._completer.setCaseSensitivity(Qt.CaseSensitivity.CaseInsensitive) + self._completer.setMaxVisibleItems(8) + self._completer.setWidget(self.search_edit) + self._completer.activated[QModelIndex].connect(self._on_suggestion_chosen) + + self._search_timer = QTimer(self) + self._search_timer.setSingleShot(True) + self._search_timer.setInterval(_SEARCH_DEBOUNCE_MS) + self._search_timer.timeout.connect(self._on_search) self.map_view = SlippyMapWidget() self.map_view.pin_moved.connect(self._on_pin_moved) @@ -181,28 +193,39 @@ def location(self) -> tuple[Optional[float], Optional[float], str, str, str]: # ── search ─────────────────────────────────────────────────────────── + def _on_search_text_edited(self, text: str) -> None: + if len(text.strip()) < _MIN_QUERY_CHARS: + self._search_timer.stop() + return + self._search_timer.start() + def _on_search(self) -> None: + self._search_timer.stop() query = self.search_edit.text().strip() - if not query: + if len(query) < _MIN_QUERY_CHARS: return self.status_label.setText("Searching…") self._pool.start(_SearchJob(self._signals, query)) def _on_search_done(self, results: object) -> None: - self._results = list(results) if isinstance(results, list) else [] - self.results_list.blockSignals(True) - self.results_list.clear() - for item in self._results: - self.results_list.addItem(str(item.get("display_name", ""))) - self.results_list.setCurrentRow(-1) - self.results_list.blockSignals(False) - self.results_list.setVisible(bool(self._results)) - self.status_label.setText("" if self._results else _OFFLINE_HINT) - - def _on_result_selected(self, row: int) -> None: - if not 0 <= row < len(self._results): + self._results = {} + for item in results if isinstance(results, list) else []: + name = str(item.get("display_name", "")) + if name and name not in self._results: + self._results[name] = item + + self._suggestions.setStringList(list(self._results)) + if self._results: + self._completer.complete() + self.status_label.setText("") + else: + self._completer.popup().hide() + self.status_label.setText(_NO_MATCH_HINT) + + def _on_suggestion_chosen(self, index: QModelIndex) -> None: + result = self._results.get(str(index.data())) + if result is None: return - result = self._results[row] coords = result_coords(result) if coords is None: return diff --git a/negpy/services/maps.py b/negpy/services/maps.py index 8e09aa8c..4706f44f 100644 --- a/negpy/services/maps.py +++ b/negpy/services/maps.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import locale import os import urllib.parse import urllib.request @@ -28,6 +29,13 @@ # The OSM tile policy requires an identifying User-Agent and local caching. _USER_AGENT = f"NegPy/{get_app_version()} (+https://github.com/marcinz606/NegPy)" + +def accept_language() -> str: + """Ask for place names in the user's language: an unreadable script would be written to XMP.""" + tag = (locale.getlocale()[0] or os.environ.get("LANG", "")).split(".")[0].replace("_", "-") + return f"{tag},en" if tag else "en" + + _CITY_KEYS = ("city", "town", "village", "hamlet", "municipality", "suburb") _STATE_KEYS = ("state", "region", "province", "county") @@ -87,14 +95,25 @@ def search_places(query: str, limit: int = 8, timeout: float = _TIMEOUT) -> list """Nominatim hits for a place name. Empty when offline or nothing matches.""" if not query.strip(): return [] - params = urllib.parse.urlencode({"q": query.strip(), "format": "json", "addressdetails": 1, "limit": limit}) + params = urllib.parse.urlencode( + {"q": query.strip(), "format": "json", "addressdetails": 1, "limit": limit, "accept-language": accept_language()} + ) payload = _json(f"{_NOMINATIM}/search?{params}", timeout) return [item for item in payload if isinstance(item, dict)] if isinstance(payload, list) else [] def reverse_place(lat: float, lon: float, timeout: float = _TIMEOUT) -> Optional[dict]: """The Nominatim result for a position, or None when it cannot be reached.""" - params = urllib.parse.urlencode({"lat": f"{lat:.6f}", "lon": f"{lon:.6f}", "format": "json", "addressdetails": 1, "zoom": 10}) + params = urllib.parse.urlencode( + { + "lat": f"{lat:.6f}", + "lon": f"{lon:.6f}", + "format": "json", + "addressdetails": 1, + "zoom": 10, + "accept-language": accept_language(), + } + ) payload = _json(f"{_NOMINATIM}/reverse?{params}", timeout) return payload if isinstance(payload, dict) and "error" not in payload else None diff --git a/tests/test_location_picker_dialog.py b/tests/test_location_picker_dialog.py index af455e3e..7595f899 100644 --- a/tests/test_location_picker_dialog.py +++ b/tests/test_location_picker_dialog.py @@ -5,6 +5,7 @@ os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") +from PyQt6.QtCore import QModelIndex # noqa: E402 from PyQt6.QtWidgets import QApplication # noqa: E402 from negpy.desktop.view.widgets.location_picker_dialog import LocationPickerDialog # noqa: E402 @@ -40,20 +41,78 @@ def test_opens_with_the_existing_location(monkeypatch) -> None: assert dlg.map_view.pin() == (35.6586, 139.7454) +def _suggestions(dlg: LocationPickerDialog) -> list[str]: + return dlg._suggestions.stringList() + + +def _choose(dlg: LocationPickerDialog, row: int) -> None: + dlg._on_suggestion_chosen(dlg._suggestions.index(row, 0)) + + def test_search_result_sets_pin_and_place(monkeypatch) -> None: dlg = _dialog(monkeypatch) dlg._on_search_done([_TOKYO]) - assert dlg.results_list.isVisible() is False or dlg.results_list.count() == 1 - dlg._on_result_selected(0) + assert _suggestions(dlg) == ["Tokyo, Japan"] + _choose(dlg, 0) lat, lon, city, state, country = dlg.location() assert (round(lat, 4), round(lon, 4)) == (35.6762, 139.6503) assert (city, state, country) == ("Tokyo", "Tokyo", "Japan") -def test_empty_search_result_reports_unavailable(monkeypatch) -> None: +def test_empty_search_result_clears_the_suggestions(monkeypatch) -> None: dlg = _dialog(monkeypatch) + dlg._on_search_done([_TOKYO]) dlg._on_search_done([]) - assert "unavailable" in dlg.status_label.text() + assert _suggestions(dlg) == [] + assert "No place matched" in dlg.status_label.text() + + +def test_typing_searches_after_a_pause_and_not_per_keystroke(monkeypatch) -> None: + """Nominatim allows one request a second, so the timer must absorb the keystrokes.""" + queries: list[str] = [] + monkeypatch.setattr( + "negpy.desktop.view.widgets.location_picker_dialog.search_places", + lambda query, **_kwargs: queries.append(query) or [], + ) + dlg = _dialog(monkeypatch) + + for text in ("t", "to", "tok"): + dlg.search_edit.setText(text) + dlg._on_search_text_edited(text) + + assert queries == [] + assert dlg._search_timer.isActive() is True + + dlg._search_timer.timeout.emit() + assert queries == ["tok"] + + +def test_a_too_short_query_never_searches(monkeypatch) -> None: + queries: list[str] = [] + monkeypatch.setattr( + "negpy.desktop.view.widgets.location_picker_dialog.search_places", + lambda query, **_kwargs: queries.append(query) or [], + ) + dlg = _dialog(monkeypatch) + dlg.search_edit.setText("to") + dlg._on_search_text_edited("to") + assert dlg._search_timer.isActive() is False + + dlg._on_search() + assert queries == [] + + +def test_duplicate_display_names_collapse_to_one_suggestion(monkeypatch) -> None: + dlg = _dialog(monkeypatch) + dlg._on_search_done([_TOKYO, dict(_TOKYO)]) + assert _suggestions(dlg) == ["Tokyo, Japan"] + + +def test_choosing_nothing_leaves_the_place_alone(monkeypatch) -> None: + dlg = _dialog(monkeypatch, city="Kyoto") + dlg._on_search_done([_TOKYO]) + dlg._on_suggestion_chosen(QModelIndex()) + assert dlg.location()[2] == "Kyoto" def test_pasted_map_link_moves_the_pin(monkeypatch) -> None: diff --git a/tests/test_maps.py b/tests/test_maps.py index 00b4c153..2c900d95 100644 --- a/tests/test_maps.py +++ b/tests/test_maps.py @@ -112,3 +112,24 @@ def test_offline_returns_none(self, monkeypatch, tmp_path) -> None: monkeypatch.setattr(maps, "tile_cache_path", lambda z, x, y: str(tmp_path / "t.png")) _patch_urlopen(monkeypatch, None) assert maps.fetch_tile(9, 1, 2) is None + + +class TestAcceptLanguage: + def test_asks_for_the_locale_language_with_an_english_fallback(self, monkeypatch) -> None: + monkeypatch.setattr(maps.locale, "getlocale", lambda *a: ("pl_PL", "UTF-8")) + assert maps.accept_language() == "pl-PL,en" + + def test_falls_back_to_the_environment_then_to_english(self, monkeypatch) -> None: + monkeypatch.setattr(maps.locale, "getlocale", lambda *a: (None, None)) + monkeypatch.setenv("LANG", "ja_JP.UTF-8") + assert maps.accept_language() == "ja-JP,en" + + monkeypatch.setenv("LANG", "") + assert maps.accept_language() == "en" + + def test_both_lookups_send_it(self, monkeypatch) -> None: + monkeypatch.setattr(maps, "accept_language", lambda: "pl-PL,en") + urls = _patch_urlopen(monkeypatch, []) + maps.search_places("Tokyo") + maps.reverse_place(35.0, 139.0) + assert all("accept-language=pl-PL%2Cen" in url for url in urls) From a92e078cb3600e48c3c33d1e55a64beeb31a4846 Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Wed, 19 Aug 2026 18:36:15 +0200 Subject: [PATCH 3/4] refactor(metadata): move the batch sync toggle out of the Scanning card It applies to every field in the panel, not to scanning, and it was folded away in a collapsed card. It now sits under Protect original metadata, where the other panel-wide switch is, and greys out with the rest when Protect is on, which makes the panel's fields moot anyway. The sync path itself needed nothing: all three call sites hand over the active frame's MetadataConfig whole, so the capture date and place ride along with the gear. A test pins that, since a future narrowing to a field list would silently drop whatever was added last. --- docs/USER_GUIDE.md | 2 +- negpy/desktop/view/sidebar/metadata.py | 11 +++-- tests/test_batch_metadata_sync.py | 64 ++++++++++++++++++++++++++ tests/test_metadata_sidebar.py | 22 ++++++++- 4 files changed, 94 insertions(+), 5 deletions(-) create mode 100644 tests/test_batch_metadata_sync.py diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index af8fca38..3d7ed1cb 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -814,6 +814,7 @@ The primary **Export** action. Its chevron menu picks the scope: current frame ( Archival metadata for the **original analog capture** (camera, lens, film, process), written into exported files as EXIF and embedded XMP, so DAMs like Lightroom show your film gear rather than the scanner. * **Protect original metadata**: copy the source file's EXIF/XMP to exports unchanged, adding nothing. When it is on, the fields below are ignored. +* **Sync custom metadata to all files in batch export**: batch and preset exports write this frame's capture, gear and process values to every file, instead of each file's own. **Analog Gear** (searchable; type in any field to filter the library): @@ -836,7 +837,6 @@ Archival metadata for the **original analog capture** (camera, lens, film, proce * **Scanning**: scan method or notes. EXIF `Software` is always `NegPy`. * **Roll / Frame**: Scanlight capture roll name and frame number, stamped automatically on capture and editable here. Available in export filename templates as `{{ roll }}` and `{{ frame }}`, and written to XMP as `negpy:CaptureRoll` and `negpy:CaptureFrame` when set. Not the Roll Analysis normalization name. -* **Sync custom metadata to all files in batch export**: apply this tab's values to every file in a batch. **Exposure**: optional original shutter, aperture and ISO. Click the lock to edit a free-text string, for example `1/125s f/2.8 ISO 400`. diff --git a/negpy/desktop/view/sidebar/metadata.py b/negpy/desktop/view/sidebar/metadata.py index 4f6ec80c..0fcb6c06 100644 --- a/negpy/desktop/view/sidebar/metadata.py +++ b/negpy/desktop/view/sidebar/metadata.py @@ -71,6 +71,13 @@ def _init_ui(self) -> None: ) self.layout.addWidget(self.protect_check) + self.sync_check = QCheckBox("Sync custom metadata to all files in batch export") + self.sync_check.setChecked(conf.sync_to_batch) + self.sync_check.setToolTip( + "Batch and preset exports write this frame's capture, gear and process values to every file, instead of each file's own." + ) + self.layout.addWidget(self.sync_check) + self._metadata_controls = QWidget() controls = QVBoxLayout(self._metadata_controls) controls.setContentsMargins(0, 0, 0, 0) @@ -199,9 +206,6 @@ def _init_ui(self) -> None: roll_row.addLayout(frame_col, 1) scan.addLayout(roll_row) - self.sync_check = QCheckBox("Sync custom metadata to all files in batch export") - self.sync_check.setChecked(conf.sync_to_batch) - scan.addWidget(self.sync_check) controls.addWidget(self._card("Scanning", "scanning", scan_body, "mdi6.scanner")) # ── EXPOSURE ───────────────────────────────────────────────────── @@ -286,6 +290,7 @@ def _make_exif_field(self, key: str, layout: QVBoxLayout) -> QLineEdit: def _set_metadata_controls_enabled(self, enabled: bool) -> None: self._metadata_controls.setEnabled(enabled) self.description_fields_btn.setEnabled(enabled) + self.sync_check.setEnabled(enabled) def _apply_lock_style(self, edit: QLineEdit, locked: bool) -> None: if locked: diff --git a/tests/test_batch_metadata_sync.py b/tests/test_batch_metadata_sync.py new file mode 100644 index 00000000..f7142d33 --- /dev/null +++ b/tests/test_batch_metadata_sync.py @@ -0,0 +1,64 @@ +"""Sync-to-batch hands the active frame's whole MetadataConfig to every file. + +Whole object, not a field list: a metadata field added to the panel is synced without +touching this path. The test pins that, so a future narrowing shows up here. +""" + +from __future__ import annotations + +from dataclasses import fields, replace +from types import SimpleNamespace +from unittest.mock import MagicMock + +from negpy.desktop.controller import AppController +from negpy.domain.models import WorkspaceConfig +from negpy.features.metadata.models import MetadataConfig + +_ACTIVE = MetadataConfig( + sync_to_batch=True, + capture_date="1998-07-14 16:30", + gps_latitude=35.6762, + gps_longitude=139.6503, + location_city="Tokyo", + location_state="Tokyo", + location_country="Japan", + film="Portra 400", +) +_PER_FILE = MetadataConfig(film="Velvia 50") + + +def _controller(active: MetadataConfig) -> MagicMock: + controller = MagicMock() + controller.state.config = replace(WorkspaceConfig(), metadata=active) + controller.state.current_file_hash = "not-this-one" + controller._batch_params_for.return_value = replace(WorkspaceConfig(), metadata=_PER_FILE) + controller._tasks_for_file.return_value = [] + return controller + + +def _synced_metadata(active: MetadataConfig) -> MetadataConfig: + controller = _controller(active) + AppController._build_preset_export_tasks(controller, [{"hash": "h1", "path": "/a/1.nef"}], [SimpleNamespace()]) + return controller._tasks_for_file.call_args.kwargs["metadata_config"] + + +def test_sync_on_sends_the_active_frames_config_to_every_file() -> None: + assert _synced_metadata(_ACTIVE) is _ACTIVE + + +def test_sync_off_sends_each_files_own_config() -> None: + assert _synced_metadata(replace(_ACTIVE, sync_to_batch=False)) is _PER_FILE + + +def test_every_metadata_field_is_carried() -> None: + """The active config is passed whole, so no field can be left behind.""" + synced = _synced_metadata(_ACTIVE) + for field in fields(MetadataConfig): + assert getattr(synced, field.name) == getattr(_ACTIVE, field.name) + + +def test_capture_date_and_place_reach_the_export() -> None: + synced = _synced_metadata(_ACTIVE) + assert synced.capture_date == "1998-07-14 16:30" + assert (synced.gps_latitude, synced.gps_longitude) == (35.6762, 139.6503) + assert (synced.location_city, synced.location_country) == ("Tokyo", "Japan") diff --git a/tests/test_metadata_sidebar.py b/tests/test_metadata_sidebar.py index 4284475b..447dbc04 100644 --- a/tests/test_metadata_sidebar.py +++ b/tests/test_metadata_sidebar.py @@ -12,7 +12,7 @@ import piexif import pytest -from PyQt6.QtWidgets import QApplication, QLabel +from PyQt6.QtWidgets import QApplication, QCheckBox, QLabel from conftest import FakeController from negpy.desktop.view.sidebar import metadata as metadata_module @@ -170,3 +170,23 @@ def test_an_existing_place_needs_no_centre(self, sidebar: MetadataSidebar, monke seen = self._capture_picker(monkeypatch) sidebar._open_location_picker() assert seen["center"] is None + + +class TestSyncCheckbox: + def test_sits_at_the_top_beside_protect_and_not_inside_a_card(self, sidebar: MetadataSidebar) -> None: + order = [sidebar.layout.indexOf(w) for w in (sidebar.protect_check, sidebar.sync_check)] + assert -1 not in order + assert order[0] < order[1] < sidebar.layout.indexOf(sidebar._metadata_controls) + assert sidebar._metadata_controls.findChildren(QCheckBox).count(sidebar.sync_check) == 0 + + def test_protect_disables_it(self, sidebar: MetadataSidebar) -> None: + """Protect mode ignores the panel's fields, so syncing them would mean nothing.""" + sidebar._on_protect_toggled(True) + assert sidebar.sync_check.isEnabled() is False + sidebar._on_protect_toggled(False) + assert sidebar.sync_check.isEnabled() is True + + def test_toggle_persists(self, sidebar: MetadataSidebar) -> None: + sidebar.sync_check.setChecked(True) + sidebar._persist_all_metadata_settings() + assert sidebar.state.config.metadata.sync_to_batch is True From 0e66e77e04adae0e50646105e9869d038a980389 Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Wed, 19 Aug 2026 18:48:08 +0200 Subject: [PATCH 4/4] refactor(metadata): icons for the capture place buttons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Map…" and "Clear" took most of the Place row, leaving the field it belongs to too narrow to read a coordinate pair. Both are _icon_action buttons now, the sidebar's helper for one-shot icon buttons, with the label moved into the tooltip. --- docs/USER_GUIDE.md | 2 +- negpy/desktop/view/sidebar/metadata.py | 5 ++--- tests/test_metadata_sidebar.py | 8 ++++++++ 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 3d7ed1cb..c4a557b1 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -825,7 +825,7 @@ Archival metadata for the **original analog capture** (camera, lens, film, proce **Capture:** * **Date**: when the frame was shot — give only what you know: `1998`, `1998-07`, `1998-07-14` or `1998-07-14 16:30`, with an optional offset such as `+02:00`. An impossible date turns the field red and is not saved. EXIF `DateTimeOriginal` pads the missing parts; XMP `photoshop:DateCreated` keeps the truncated form and `negpy:CaptureDatePrecision` names it. The scan file's own timestamp moves to `DateTimeDigitized`. -* **Place**: the capture location. **Map…** opens a map to search a place name, click a position or paste coordinates, **Clear** empties it, and the field itself accepts a pasted coordinate pair or an OpenStreetMap/Google Maps link. Coordinates are written to the EXIF GPS tags and XMP `exif:GPS*`, the names to XMP `photoshop:City`/`State`/`Country`; a TIFF carries the location in XMP only, and a place you set replaces the source file's GPS block whole, rather than leaving its altitude or heading beside your coordinates. A geotagged source with no place set here keeps its own coordinates on export, and the map opens centred on them — where the frame was digitized is a starting view, never the capture place. Opening the map contacts OpenStreetMap; typing coordinates needs no network. +* **Place**: the capture location. The map-pin button opens a map to search a place name, click a position or paste coordinates, the ✕ beside it empties the place, and the field itself accepts a pasted coordinate pair or an OpenStreetMap/Google Maps link. Coordinates are written to the EXIF GPS tags and XMP `exif:GPS*`, the names to XMP `photoshop:City`/`State`/`Country`; a TIFF carries the location in XMP only, and a place you set replaces the source file's GPS block whole, rather than leaving its altitude or heading beside your coordinates. A geotagged source with no place set here keeps its own coordinates on export, and the map opens centred on them — where the frame was digitized is a starting view, never the capture place. Opening the map contacts OpenStreetMap; typing coordinates needs no network. **Process:** diff --git a/negpy/desktop/view/sidebar/metadata.py b/negpy/desktop/view/sidebar/metadata.py index 0fcb6c06..40684c6e 100644 --- a/negpy/desktop/view/sidebar/metadata.py +++ b/negpy/desktop/view/sidebar/metadata.py @@ -138,10 +138,9 @@ def _init_ui(self) -> None: self.place_edit.setPlaceholderText("Pick on a map, or paste coordinates") self.place_edit.setToolTip("Capture place. Paste a coordinate pair or a map link here, or use Map… to pick one.") place_row.addWidget(self.place_edit, 1) - self.place_map_btn = QPushButton("Map…") - self.place_map_btn.setToolTip("Pick the capture place on a map (contacts OpenStreetMap)") + self.place_map_btn = self._icon_action("fa5s.map-marked-alt", "Pick the capture place on a map (contacts OpenStreetMap)") place_row.addWidget(self.place_map_btn) - self.place_clear_btn = QPushButton("Clear") + self.place_clear_btn = self._icon_action("fa5s.times", "Clear the capture place") place_row.addWidget(self.place_clear_btn) cap.addLayout(place_row) controls.addWidget(self._card("Capture", "capture", cap_body, "fa5s.clock")) diff --git a/tests/test_metadata_sidebar.py b/tests/test_metadata_sidebar.py index 447dbc04..f0e90ce6 100644 --- a/tests/test_metadata_sidebar.py +++ b/tests/test_metadata_sidebar.py @@ -190,3 +190,11 @@ def test_toggle_persists(self, sidebar: MetadataSidebar) -> None: sidebar.sync_check.setChecked(True) sidebar._persist_all_metadata_settings() assert sidebar.state.config.metadata.sync_to_batch is True + + +class TestPlaceButtons: + def test_are_icon_only_with_tooltips_carrying_the_meaning(self, sidebar: MetadataSidebar) -> None: + for button in (sidebar.place_map_btn, sidebar.place_clear_btn): + assert button.text() == "" + assert button.icon().isNull() is False + assert button.toolTip() != ""