[feat] save drift corrector images#3487
Conversation
| 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: |
There was a problem hiding this comment.
@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?
📝 WalkthroughWalkthroughThis PR adds optional drift corrector image saving to SPARC acquisition. A new Sequence DiagramsequenceDiagram
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()
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 | 🟠 MajorAdd 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
📒 Files selected for processing (3)
plugins/sparc_save_drift_corrector_images.pysrc/odemis/acq/drift/__init__.pysrc/odemis/acq/leech.py
| class SaveDriftCorrectorImgPlugin(Plugin): | ||
| name = "Save drift corrector images" |
There was a problem hiding this comment.
🧩 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" || trueRepository: 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" || trueRepository: 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 formenu_itemand 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
| 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) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@nandishjpatel , for clarification, you could add a comment here that states the plugins are always initialised after the rest of the GUI, so the driftCorrector is always initialised.
Alternatively, for even more confidence, you could just store the tab controller, and explicitly access the driftCorrector inside _save_drift_corrector_images()
There was a problem hiding this comment.
@pieleric, thanks for the clarification — that init-order guarantee (plugins loaded after the rest of the GUI) addresses my original concern about driftCorrector possibly being None. Storing the tab controller and accessing driftCorrector lazily inside _save_drift_corrector_images() is indeed the more robust option since it stays correct even if that ordering assumption ever changes. Either approach (a clarifying comment, or the lazy-access pattern) works well here.
✏️ Learnings added
Learnt from: pieleric
Repo: delmic/odemis PR: 3487
File: plugins/sparc_save_drift_corrector_images.py:48-52
Timestamp: 2026-07-06T19:00:37.247Z
Learning: In delmic/odemis, GUI plugins (subclasses of odemis.gui.plugin.Plugin) are always initialized after the rest of the GUI is set up, so model attributes like tab_data_model.driftCorrector on tabs (e.g., SPARC acquisition tab) are guaranteed to be already initialized by the time a plugin's __init__ runs. This means None-checks for such attributes at plugin init time are unnecessary, though storing the tab controller and accessing the attribute lazily inside callback methods is a more robust alternative if init-order assumptions might change.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: pieleric
Repo: delmic/odemis PR: 3495
File: src/odemis/gui/cont/milling.py:107-109
Timestamp: 2026-06-22T12:24:21.837Z
Learning: In delmic/odemis (Python), keep docstrings as plain text only. Do not use reStructuredText (RST) markup/directives such as ":param:", ":return:", or ":type:" in docstrings; write the documentation as unformatted text instead.
| DRIFT_IMAGES_DIR = os.path.join(get_picture_folder(), "Drift Correction images") | ||
|
|
There was a problem hiding this comment.
🧩 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" || trueRepository: 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" || trueRepository: 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 || trueRepository: 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" || trueRepository: 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
| 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. | ||
| """ |
There was a problem hiding this comment.
🧩 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 || trueRepository: 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
| self.save_images = save_images | ||
|
|
There was a problem hiding this comment.
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_imagesA 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.
There was a problem hiding this comment.
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_imagesflag toAnchorDriftCorrectorand forwards it intodrift.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=Truethe code now saves every acquired drift image to disk. Please update this comment to reflect the current behavior (the limiting logic below only applies toself.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") |
There was a problem hiding this comment.
@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
There was a problem hiding this comment.
That's actually my biggest issue with this original patch: it stores the drift data at a different place from the acquisition file, and mix it with the rest of the standard acquisitions. Instead, it could use the argument log_path= (as used in other functions), for indicating where to store (and could even include the beginning of the filename). If None, it doesn't record anything.
Then, we can implement once of the two:
- Always record in a dedicated folder in the same place as where the mirror alignment function stores the data ($HOME/odemis-status/)
- In the stream tab, when the filename (and path) is changed, the drift estimator leech
.log_pathattribute is updated. (instead of.save_images)
I would probably go for the second option, as it seems more targetted at the user than the developers, but I don't have strong opinion.
| 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) |
| # 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) |
| 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 |
| self.roi.value, | ||
| self.dwellTime.value) | ||
| self.dwellTime.value, | ||
| save_images=self.save_images) |
| 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") |
There was a problem hiding this comment.
@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
| 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) |
There was a problem hiding this comment.
@nandishjpatel , for clarification, you could add a comment here that states the plugins are always initialised after the rest of the GUI, so the driftCorrector is always initialised.
Alternatively, for even more confidence, you could just store the tab controller, and explicitly access the driftCorrector inside _save_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") |
There was a problem hiding this comment.
That's actually my biggest issue with this original patch: it stores the drift data at a different place from the acquisition file, and mix it with the rest of the standard acquisitions. Instead, it could use the argument log_path= (as used in other functions), for indicating where to store (and could even include the beginning of the filename). If None, it doesn't record anything.
Then, we can implement once of the two:
- Always record in a dedicated folder in the same place as where the mirror alignment function stores the data ($HOME/odemis-status/)
- In the stream tab, when the filename (and path) is changed, the drift estimator leech
.log_pathattribute is updated. (instead of.save_images)
I would probably go for the second option, as it seems more targetted at the user than the developers, but I don't have strong opinion.
No description provided.