Skip to content

[feat] save drift corrector images#3487

Open
nandishjpatel wants to merge 1 commit into
delmic:masterfrom
nandishjpatel:drift-alignment-correction
Open

[feat] save drift corrector images#3487
nandishjpatel wants to merge 1 commit into
delmic:masterfrom
nandishjpatel:drift-alignment-correction

Conversation

@nandishjpatel

Copy link
Copy Markdown
Contributor

No description provided.

logging.warning("Shape of data is %s instead of %s", data.shape[::-1], self._res)

# Save all the drift region scans for offline autocorrelation purposes
if self._save_images:

@nandishjpatel nandishjpatel Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@pieleric The changes in src/odemis/acq/drift/init.py are made by @yxkdejong. I am not 100% sure if this is what AMOLF needs. Since you have previously written code for the drift corrector, can you tell?

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds optional drift corrector image saving to SPARC acquisition. A new save_images boolean parameter flows through the acquisition stack: the GUI plugin provides menu control to toggle it on the drift corrector model, the controller passes it to the estimator, and the estimator conditionally saves each anchor scan as a .tif file with session-scoped incrementing filenames, creating the output directory as needed.

Sequence Diagram

sequenceDiagram
  participant User
  participant SaveDriftCorrectorImgPlugin
  participant DriftCorrector
  participant AnchorDriftCorrector
  participant AnchoredEstimator
  User->>SaveDriftCorrectorImgPlugin: Toggle menu item
  SaveDriftCorrectorImgPlugin->>DriftCorrector: Set save_images property
  Note over DriftCorrector: Menu state determines<br/>save_images value
  DriftCorrector->>AnchorDriftCorrector: Constructor receives<br/>save_images
  AnchorDriftCorrector->>AnchoredEstimator: Pass save_images parameter
  Note over AnchoredEstimator: Saves anchor scans to disk<br/>during acquire()
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess whether the description is related to the changeset. Please provide a pull request description that explains the purpose and motivation for adding the drift corrector image saving functionality.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title '[feat] save drift corrector images' directly summarizes the main feature addition - enabling saving of drift corrector images, which is implemented across the plugin and drift correction modules.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/odemis/acq/drift/__init__.py (1)

53-65: 🛠️ Refactor suggestion | 🟠 Major

Add type hints and fix reStructuredText docstring for AnchoredEstimator.__init__.

In src/odemis/acq/drift/__init__.py (lines 53-65), __init__ has untyped parameters/return and the docstring includes inline type info (e.g., region (4 floats), max_pixels (int):, follow_drift (bool):). Update the constructor signature with type hints and rewrite the docstring to the required reStructuredText style without inline type annotations.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/odemis/acq/drift/__init__.py` around lines 53 - 65, Update
AnchoredEstimator.__init__ to use Python type hints for all parameters (e.g.,
scanner: Emitter, detector: Detector, region: Sequence[float] or Tuple[float,
float, float, float], dwell_time: float, max_pixels: int = MAX_PIXELS,
follow_drift: bool = True, save_images: bool = False) and return type None; then
replace the inline-typed docstring with reStructuredText style parameter
descriptions (use :param scanner:, :type scanner:, :param detector:, :type
detector:, :param region:, :type region:, :param dwell_time:, :type dwell_time:,
:param max_pixels:, :type max_pixels:, :param follow_drift:, :type
follow_drift:, :param save_images:, :type save_images:) and short explanatory
text for each parameter, keeping the note about anchor region resizing and
follow_drift behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@plugins/sparc_save_drift_corrector_images.py`:
- Around line 48-52: The menu callback is wired even if
sparc_acq_tab.tab_data_model.driftCorrector may be None, causing runtime errors
when _save_drift_corrector_images uses self._drift_corrector; fix by guarding
when creating the menu: check sparc_acq_tab.tab_data_model.driftCorrector and
only call addMenu (or disable the menu item) if it's not None, and in
_save_drift_corrector_images defensively handle a None self._drift_corrector
(e.g., show a warning and return) so both initialization-time None and later
changes are safe; update references to self._drift_corrector, addMenu,
_save_drift_corrector_images and the source
sparc_acq_tab.tab_data_model.driftCorrector accordingly.
- Around line 30-31: The SaveDriftCorrectorImgPlugin class lacks a class rST
docstring and its methods lack type hints/docstrings; add an rST-style class
docstring to SaveDriftCorrectorImgPlugin describing its purpose, then update
__init(self, microscope, main_app) to include proper type hints for microscope
and main_app (and -> None) and add a short rST docstring describing parameters,
types, and return; also add type hints to _save_drift_corrector_images(self,
menu_item) (typing for menu_item and -> None) and ensure its existing docstring
is converted/expanded to rST param/return sections. Reference the class name
SaveDriftCorrectorImgPlugin and the methods __init__ and
_save_drift_corrector_images when making these changes.

In `@src/odemis/acq/drift/__init__.py`:
- Around line 39-40: Replace os.path and os.makedirs usage for drift image paths
with pathlib.Path: construct DRIFT_IMAGES_DIR using Path(get_picture_folder()) /
"Drift Correction images", ensure directories are created via
Path.mkdir(parents=True, exist_ok=True) where the code currently calls
os.makedirs, and update any image write calls (cv2.imwrite) to pass
str(filename) when filename is a Path object; search for DRIFT_IMAGES_DIR,
get_picture_folder(), and cv2.imwrite in this module to locate the three spots
to change.

In `@src/odemis/acq/leech.py`:
- Around line 155-156: The issue: runtime changes to the plugin's save_images
flag are not propagated to the active AnchoredEstimator because the value is
only copied in series_start(); fix by adding a public setter on
AnchoredEstimator (e.g., set_save_images or update_save_images) and call that
setter wherever the code currently assigns self.save_images into the estimator
(notably in series_start() and the other similar locations around where
save_images is copied), rather than directly mutating a private field; ensure
AnchoredEstimator.acquire() reads the public/internal field the setter updates
so toggles take effect immediately.
- Around line 144-149: Update AnchorDriftCorrector.__init__ to use Python type
hints: annotate parameters as scanner: Emitter, detector: Detector, save_images:
bool = False and add a -> None return annotation; then remove the inline type
markers from the reStructuredText docstring so the :param entries only describe
the parameter purpose (e.g., ":param scanner: A component with a .dwellTime,
.translation, .scale."). Keep the same parameter names and preserve existing
descriptions for scanner, detector, and save_images.

---

Outside diff comments:
In `@src/odemis/acq/drift/__init__.py`:
- Around line 53-65: Update AnchoredEstimator.__init__ to use Python type hints
for all parameters (e.g., scanner: Emitter, detector: Detector, region:
Sequence[float] or Tuple[float, float, float, float], dwell_time: float,
max_pixels: int = MAX_PIXELS, follow_drift: bool = True, save_images: bool =
False) and return type None; then replace the inline-typed docstring with
reStructuredText style parameter descriptions (use :param scanner:, :type
scanner:, :param detector:, :type detector:, :param region:, :type region:,
:param dwell_time:, :type dwell_time:, :param max_pixels:, :type max_pixels:,
:param follow_drift:, :type follow_drift:, :param save_images:, :type
save_images:) and short explanatory text for each parameter, keeping the note
about anchor region resizing and follow_drift behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e0878f3b-d7de-4a2d-9d41-07a44ad2d931

📥 Commits

Reviewing files that changed from the base of the PR and between f74a181 and 7401f14.

📒 Files selected for processing (3)
  • plugins/sparc_save_drift_corrector_images.py
  • src/odemis/acq/drift/__init__.py
  • src/odemis/acq/leech.py

Comment on lines +30 to +31
class SaveDriftCorrectorImgPlugin(Plugin):
name = "Save drift corrector images"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show relevant portions of the file around the referenced lines
FILE="plugins/sparc_save_drift_corrector_images.py"
ls -la "$FILE" || true
nl -ba "$FILE" | sed -n '1,120p'
nl -ba "$FILE" | sed -n '120,240p'
nl -ba "$FILE" | sed -n '240,360p'

# Quick structural search for type hints and docstrings in the plugin file
rg -n "class SaveDriftCorrectorImgPlugin|def " "$FILE" || true
rg -n "->|: " "$FILE" || true
rg -n '"""|\\\'\\\'\\\'' "$FILE" || true

Repository: delmic/odemis

Length of output: 212


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="plugins/sparc_save_drift_corrector_images.py"
echo "== File =="; ls -la "$FILE"

echo "== Lines 1-120 (for context) =="
awk 'NR<=120{printf "%6d:%s\n", NR, $0}' "$FILE"

echo "== Focused ranges =="
for start in 25 30 34 36 48 54; do
  end=$((start+15))
  echo "--- Lines ${start}-${end} ---"
  awk -v s="$start" -v e="$end" 'NR>=s && NR<=e{printf "%6d:%s\n", NR, $0}' "$FILE"
done

echo "== Search: class/defs ==" 
rg -n "class |def " "$FILE" || true

echo "== Search: type hints (-> or annotated params) =="
rg -n "->|def .*:.*|def .*: .*=" "$FILE" || true

echo "== Search: docstrings ==" 
rg -n '"""|'\'''\''' "$FILE" || true

Repository: delmic/odemis

Length of output: 8063


Add type hints and rST docstrings to the new SaveDriftCorrectorImgPlugin

File: plugins/sparc_save_drift_corrector_images.py

  • SaveDriftCorrectorImgPlugin (lines 30-31) is missing a class docstring.
  • __init__(self, microscope, main_app) (lines 36-37) is missing type hints for parameters/return type and has no docstring.
  • _save_drift_corrector_images(self, menu_item) (lines 54-62) is missing type hints for menu_item and the return type (it already has a docstring).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@plugins/sparc_save_drift_corrector_images.py` around lines 30 - 31, The
SaveDriftCorrectorImgPlugin class lacks a class rST docstring and its methods
lack type hints/docstrings; add an rST-style class docstring to
SaveDriftCorrectorImgPlugin describing its purpose, then update __init(self,
microscope, main_app) to include proper type hints for microscope and main_app
(and -> None) and add a short rST docstring describing parameters, types, and
return; also add type hints to _save_drift_corrector_images(self, menu_item)
(typing for menu_item and -> None) and ensure its existing docstring is
converted/expanded to rST param/return sections. Reference the class name
SaveDriftCorrectorImgPlugin and the methods __init__ and
_save_drift_corrector_images when making these changes.

Source: Coding guidelines

Comment on lines +48 to +52
self._drift_corrector = sparc_acq_tab.tab_data_model.driftCorrector
self.addMenu("Help/Development/Save drift corrector images",
self._save_drift_corrector_images,
item_kind=wx.ITEM_CHECK,
pass_menu_item=True)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard against None drift corrector before wiring the menu callback.

self._drift_corrector is used unconditionally in the callback. If sparc_acq_tab.tab_data_model.driftCorrector is still None at plugin init time, menu interaction will raise at runtime.

Suggested patch
         self._drift_corrector = sparc_acq_tab.tab_data_model.driftCorrector
+        if self._drift_corrector is None:
+            logging.debug("Not loading save drift corrector images tool since drift corrector is not initialized.")
+            return
         self.addMenu("Help/Development/Save drift corrector images",
                      self._save_drift_corrector_images,
                      item_kind=wx.ITEM_CHECK,
                      pass_menu_item=True)

Also applies to: 54-62

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@plugins/sparc_save_drift_corrector_images.py` around lines 48 - 52, The menu
callback is wired even if sparc_acq_tab.tab_data_model.driftCorrector may be
None, causing runtime errors when _save_drift_corrector_images uses
self._drift_corrector; fix by guarding when creating the menu: check
sparc_acq_tab.tab_data_model.driftCorrector and only call addMenu (or disable
the menu item) if it's not None, and in _save_drift_corrector_images defensively
handle a None self._drift_corrector (e.g., show a warning and return) so both
initialization-time None and later changes are safe; update references to
self._drift_corrector, addMenu, _save_drift_corrector_images and the source
sparc_acq_tab.tab_data_model.driftCorrector accordingly.

Comment on lines +39 to +40
DRIFT_IMAGES_DIR = os.path.join(get_picture_folder(), "Drift Correction images")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="src/odemis/acq/drift/__init__.py"

# Show the referenced regions with line numbers
nl -ba "$FILE" | sed -n '1,220p' | sed -n '25,110p'
echo "----"
nl -ba "$FILE" | sed -n '110,220p'
echo "----"

# Focus around the specific line numbers mentioned (39-40, 75-75, 158-158)
for L in 39 40 75 158; do
  echo "### Context around line $L"
  nl -ba "$FILE" | sed -n "$((L-5)),$((L+5))p"
done

# Search for os.path / Path usage in this module
echo "---- rg os.path"
rg -n "os\.path|os\.makedirs|os\.path\.join|Path\(" "$FILE" || true

Repository: delmic/odemis

Length of output: 100


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="src/odemis/acq/drift/__init__.py"

echo "== File stats =="
wc -l "$FILE"

echo "== Context near lines 25-110 =="
sed -n '25,110p' "$FILE" | awk '{printf "%5d:%s\n", NR+24, $0}'

echo "== Context near lines 110-220 =="
sed -n '110,220p' "$FILE" | awk '{printf "%5d:%s\n", NR+109, $0}'

echo "== Context near referenced lines =="
for L in 39 40 75 158; do
  START=$((L-5)); END=$((L+5))
  echo "--- Around line $L (showing $START-$END) ---"
  sed -n "${START},${END}p" "$FILE" | awk -v off="$START" '{printf "%5d:%s\n", off+NR-1, $0}'
done

echo "== Search for os.path / Path usage in this module =="
rg -n "os\.path|os\.makedirs|os\.path\.join|Path\(" "$FILE" || true

Repository: delmic/odemis

Length of output: 13215


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "DRIFT_IMAGES_DIR" -S src/odemis/acq/drift/__init__.py
rg -n "DRIFT_IMAGES_DIR" -S src/odemis | head -n 50
rg -n "from pathlib import Path|pathlib\.Path|Path\(" -S src/odemis/acq/drift/__init__.py || true

Repository: delmic/odemis

Length of output: 686


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="src/odemis/acq/drift/__init__.py"

echo "== rg 'os\\.' in module =="
rg -n "os\." "$FILE" || true

echo "== rg 'import os' in module =="
rg -n "^\s*import os\s*$|from os import" "$FILE" || true

echo "== Imports at top (first 60 lines) =="
sed -n '1,70p' "$FILE" | cat -n

echo "== rg datetime usage / import =="
rg -n "datetime" "$FILE" || true

Repository: delmic/odemis

Length of output: 3617


Use pathlib.Path for the drift-image output path construction.

In src/odemis/acq/drift/__init__.py, replace the os.path/os.makedirs usage at lines 39, 75, and 158 with pathlib.Path (including passing str(filename) to cv2.imwrite).

Suggested patch
-import os
+from pathlib import Path
@@
-DRIFT_IMAGES_DIR = os.path.join(get_picture_folder(), "Drift Correction images")
+DRIFT_IMAGES_DIR = Path(get_picture_folder()) / "Drift Correction images"
@@
-            os.makedirs(DRIFT_IMAGES_DIR, exist_ok=True)
+            DRIFT_IMAGES_DIR.mkdir(parents=True, exist_ok=True)
@@
-                filename = os.path.join(DRIFT_IMAGES_DIR, f"drift_{self._session_id}_{self._image_counter:05d}.tif")
+                filename = DRIFT_IMAGES_DIR / f"drift_{self._session_id}_{self._image_counter:05d}.tif"
@@
-                success = cv2.imwrite(filename, data)
+                success = cv2.imwrite(str(filename), data)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/odemis/acq/drift/__init__.py` around lines 39 - 40, Replace os.path and
os.makedirs usage for drift image paths with pathlib.Path: construct
DRIFT_IMAGES_DIR using Path(get_picture_folder()) / "Drift Correction images",
ensure directories are created via Path.mkdir(parents=True, exist_ok=True) where
the code currently calls os.makedirs, and update any image write calls
(cv2.imwrite) to pass str(filename) when filename is a Path object; search for
DRIFT_IMAGES_DIR, get_picture_folder(), and cv2.imwrite in this module to locate
the three spots to change.

Source: Coding guidelines

Comment thread src/odemis/acq/leech.py
Comment on lines +144 to 149
def __init__(self, scanner, detector, save_images=False):
"""
:param scanner: (Emitter) A component with a .dwellTime, .translation, .scale.
:param detector: (Detector) To acquire the signal.
:param save_images: (bool) Whether to save drift corrector images.
"""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="src/odemis/acq/leech.py"

echo "=== Lines around 130-190 in $FILE ==="
sed -n '120,190p' "$FILE" | cat -n

echo
echo "=== Find AnchorDriftCorrector definition ==="
rg -n "class\s+AnchorDriftCorrector\b|def\s+__init__\b" "$FILE"

echo
echo "=== Show full __init__ docstring for AnchorDriftCorrector (if present) ==="
python3 - <<'PY'
import re, itertools, pathlib
path = pathlib.Path("src/odemis/acq/leech.py")
text = path.read_text(encoding="utf-8")
# crude extraction: find class AnchorDriftCorrector then next def __init__ block docstring until next def/class at same indent
m = re.search(r"class\s+AnchorDriftCorrector\b[\s\S]*?\n(?P<body>(?:[ \t]+.*\n)*)", text)
print("Found AnchorDriftCorrector:", bool(m))
# Print around the first __init__ after class
lines = text.splitlines(True)
start=None
for i,l in enumerate(lines):
    if re.search(r"class\s+AnchorDriftCorrector\b", l):
        start=i
        break
if start is None:
    raise SystemExit("AnchorDriftCorrector not found")
# find __init__ at indent 4 after class
for j in range(start+1, len(lines)):
    if re.search(r"^ {4}def\s+__init__\b", lines[j]):
        init_start=j
        break
else:
    raise SystemExit("__init__ not found")
# read until next def/class at indent 4 or until EOF
end=init_start+1
for k in range(init_start+1, len(lines)):
    if re.search(r"^ {4}(def|class)\s+\w+", lines[k]):
        end=k
        break
# print range
for i in range(init_start, min(end, init_start+120)):
    print(f"{init_start+1+i-init_start:6d}: {lines[i].rstrip()}")
PY

echo
echo "=== Check for repo docstring pattern in this file (first few :param occurrences) ==="
rg -n ":\s*param\s+\w+\s*:" "$FILE" | head -n 20 || true

Repository: delmic/odemis

Length of output: 7257


Type-annotate AnchorDriftCorrector.__init__ and fix its reStructuredText docstring parameter formatting.

In src/odemis/acq/leech.py (lines 144-149), AnchorDriftCorrector.__init__(self, scanner, detector, save_images=False) is untyped, and the docstring embeds explicit type markers in :param entries (e.g., (Emitter), (Detector), (bool)). Add Python type hints for parameters (and -> None) and adjust :param text to follow the repo docstring rule (no type info in the docstring).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/odemis/acq/leech.py` around lines 144 - 149, Update
AnchorDriftCorrector.__init__ to use Python type hints: annotate parameters as
scanner: Emitter, detector: Detector, save_images: bool = False and add a ->
None return annotation; then remove the inline type markers from the
reStructuredText docstring so the :param entries only describe the parameter
purpose (e.g., ":param scanner: A component with a .dwellTime, .translation,
.scale."). Keep the same parameter names and preserve existing descriptions for
scanner, detector, and save_images.

Source: Coding guidelines

Comment thread src/odemis/acq/leech.py
Comment on lines +155 to 156
self.save_images = save_images

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Runtime menu toggles won’t affect the active estimator instance.

save_images is copied into AnchoredEstimator only in series_start(). If the plugin toggles self.save_images during an ongoing series, self._dc_estimator.acquire() still uses the old flag, so behavior won’t change until a new series starts.

Implementation direction
 class AnchorDriftCorrector(LeechAcquirer):
@@
     def next(self, das):
@@
+        # Keep estimator behavior in sync with runtime UI toggle.
+        if self._dc_estimator is not None and self._dc_estimator._save_images != self.save_images:
+            self._dc_estimator._save_images = self.save_images

A cleaner variant is adding a public setter on AnchoredEstimator (instead of touching a private field) and calling it here.

Also applies to: 282-287, 347-349

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/odemis/acq/leech.py` around lines 155 - 156, The issue: runtime changes
to the plugin's save_images flag are not propagated to the active
AnchoredEstimator because the value is only copied in series_start(); fix by
adding a public setter on AnchoredEstimator (e.g., set_save_images or
update_save_images) and call that setter wherever the code currently assigns
self.save_images into the estimator (notably in series_start() and the other
similar locations around where save_images is copied), rather than directly
mutating a private field; ensure AnchoredEstimator.acquire() reads the
public/internal field the setter updates so toggles take effect immediately.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds an (optional) debug/development capability to persist the anchor ROI frames used by drift correction, so they can be analyzed offline (e.g., autocorrelation).

Changes:

  • Adds a save_images flag to AnchorDriftCorrector and forwards it into drift.AnchoredEstimator.
  • Implements image writing in AnchoredEstimator.acquire() to save each drift-region scan as a TIFF.
  • Adds a SPARC GUI development plugin to toggle this behavior from the menu.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
src/odemis/acq/leech.py Adds a save_images option on the drift-corrector leech and passes it into the drift estimator.
src/odemis/acq/drift/init.py Adds optional on-disk saving of anchor ROI frames during drift acquisition.
plugins/sparc_save_drift_corrector_images.py New GUI plugin providing a checkable menu item to enable/disable saving.
Comments suppressed due to low confidence (1)

src/odemis/acq/drift/init.py:168

  • The comment says “we only save the 1st, 2nd and last two images”, but with save_images=True the code now saves every acquired drift image to disk. Please update this comment to reflect the current behavior (the limiting logic below only applies to self.raw).
            # TODO: allow to record just every Nth image, and separately record the
            # drift after every measurement
            # In the mean time, we only save the 1st, 2nd and last two images
            if len(self.raw) > 2:

MIN_RESOLUTION = (20, 20) # sometimes 8x8 works, but it's not reliable enough
MAX_PIXELS = 128 ** 2 # px

DRIFT_IMAGES_DIR = os.path.join(get_picture_folder(), "Drift Correction images")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@nandishjpatel can you look at this comment. You could also leave it completely up to the caller/plugin and add drift_images_dir as an argument to the AnchoredEstimator class

Comment on lines +70 to +75
self._save_images = save_images
self._session_id = datetime.datetime.now().strftime("%Y%m%d_%H%M%S")
self._image_counter = 0

if self._save_images:
os.makedirs(DRIFT_IMAGES_DIR, exist_ok=True)
Comment on lines +156 to +163
# Save all the drift region scans for offline autocorrelation purposes
if self._save_images:
filename = os.path.join(DRIFT_IMAGES_DIR, f"drift_{self._session_id}_{self._image_counter:05d}.tif")
self._image_counter += 1

success = cv2.imwrite(filename, data)
if not success:
logging.warning("Failed to save drift image %s", filename)
Comment thread src/odemis/acq/leech.py
self._detector = detector
self._dc_estimator = None
self._period_acq = None # number of acq left until next drift correction is performed
self.save_images = save_images
Comment thread src/odemis/acq/leech.py
self.roi.value,
self.dwellTime.value)
self.dwellTime.value,
save_images=self.save_images)
Comment on lines +57 to +62
if checked:
self._drift_corrector.save_images = True
logging.debug("Save drift corrector images checked, will acquire drift corrector images")
else:
self._drift_corrector.save_images = False
logging.debug("Save drift corrector images unchecked, will not acquire drift corrector images")
MIN_RESOLUTION = (20, 20) # sometimes 8x8 works, but it's not reliable enough
MAX_PIXELS = 128 ** 2 # px

DRIFT_IMAGES_DIR = os.path.join(get_picture_folder(), "Drift Correction images")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@nandishjpatel can you look at this comment. You could also leave it completely up to the caller/plugin and add drift_images_dir as an argument to the AnchoredEstimator class

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants