TFS scan rotation metadata support & UI fix

[tmoerkerken]

• Utilize MD_ROTATION for the TFS posture manager
• Store MD_ROTATION while acquiring in the autoscript driver
• Fix bug found during testing where a VA control represented by a log scale could have a minimum of 0 (and log(0) is undefined) --> fix: switch to cubic instead when minimum is zero or negative

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request adjusts rotation handling and related metadata across the codebase. In move.py, when a computed scan rotation (sr) is non-zero the code now calls self._set_scanner_rotation_cor(sr) during conversion setup. In the autoscript client, the per-acquisition rotation metadata key was changed from MD_BEAM_SCAN_ROTATION to MD_ROTATION, and tests were updated to expect the new key. Separately, the GUI slider adds a safety fallback: if a "log" scale is requested but the minimum value is <= 0, the scale is switched to "cubic". No public function signatures were changed.

Possibly related PRs

• [MSD-124][feature] Add scan rotation to tescan driver #3267: Related change switching rotation metadata usage to MD_ROTATION, matching this PR's metadata key update.
• [MSD-128][feature] Add scan rotation in Tescan posture manager #3294: Adds and uses _set_scanner_rotation_cor(sr) in conversion flow; directly overlaps the scanner rotation-correction call introduced here.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the two main changes: TFS scan rotation metadata support and a UI fix for invalid logarithmic scales.
Description check	✅ Passed	The description directly relates to the changeset, clearly outlining the three key modifications: MD_ROTATION metadata usage, autoscript driver storage, and the logarithmic scale bug fix.

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

✨ Finishing touches

• 📝 Generate docstrings

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

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/move.py (1)

579-587: Call to undefined _set_scanner_rotation_cor will crash when scan rotation ≠ 0

In _update_conversion, you now do:

sr = self._get_scan_rotation()
if sr != 0:
    self._set_scanner_rotation_cor(sr)

Neither MeteorPostureManager nor its subclasses in this file define _set_scanner_rotation_cor, so as soon as sr is non‑zero this will raise an AttributeError on posture managers like MeteorTFS1PostureManager / MeteorTFS3PostureManager that use the base _update_conversion.

You probably want either:

• A concrete implementation on MeteorPostureManager (or all relevant subclasses), or
• To call an existing API that performs the intended correction, instead of a method that doesn’t exist yet.

Until such a method is implemented, this side-effect should be removed or guarded behind a capability check.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8ad878c and 74abaa8.

📒 Files selected for processing (4)

• src/odemis/acq/move.py
• src/odemis/driver/autoscript_client.py
• src/odemis/driver/test/autoscript_client_test.py
• src/odemis/gui/comp/slider.py

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: build (ubuntu-24.04)
• GitHub Check: build (ubuntu-22.04)

🔇 Additional comments (2)

src/odemis/driver/autoscript_client.py (1)

1530-1541: Rotation metadata key switch to MD_ROTATION looks consistent

Storing the scanner rotation under model.MD_ROTATION alongside the other MD_* entries fits the metadata pattern and matches the updated tests. No issues spotted in this change.

src/odemis/driver/test/autoscript_client_test.py (1)

847-850: Test expectations correctly updated to MD_ROTATION

The required metadata keys now include model.MD_ROTATION, matching the driver’s metadata assignment. This keeps the detector-component test aligned with the new rotation key.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/odemis/gui/comp/slider.py (1)

229-232: Good fix for the log(0) undefined behavior.

The safety check correctly prevents the crash when a log scale slider has a minimum of 0 or negative. Switching to cubic is a reasonable fallback.

Minor observation: The condition min_val <= 0 is slightly conservative—if both min_val and max_val are negative (e.g., range [-100, -1]), the existing _log_val_to_perc can handle it via the negative-range recursion at lines 140-141. A more precise condition would be min_val <= 0 <= max_val (i.e., when the range contains or crosses zero). However, the current fix is safe and achieves the goal.

♻️ Optional: More precise condition

-        if scale == "log" and min_val <= 0:
-            logging.debug("Log scale must be strictly positive (range: %s, %s), switching to cubic", min_val, max_val)
+        if scale == "log" and min_val <= 0 <= max_val:
+            logging.debug("Log scale cannot contain 0 (range: %s, %s), switching to cubic", min_val, max_val)
             scale = "cubic" # log scale cannot have 0 in range

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f3960fd and 152e371.

📒 Files selected for processing (4)

• src/odemis/acq/move.py
• src/odemis/driver/autoscript_client.py
• src/odemis/driver/test/autoscript_client_test.py
• src/odemis/gui/comp/slider.py

🚧 Files skipped from review as they are similar to previous changes (2)

• src/odemis/driver/test/autoscript_client_test.py
• src/odemis/driver/autoscript_client.py

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2026-01-12T12:37:35.155Z

Learnt from: K4rishma
Repo: delmic/odemis PR: 3245
File: src/odemis/acq/align/z_localization.py:29-40
Timestamp: 2026-01-12T12:37:35.155Z
Learning: Maintain the debugging pattern of importing odemis.gui.conf and exporting TIFF files within acquisition/localization loops across all odemis Python sources. Do not remove or restructure this behavior in refactors if it serves debugging consistency; document the debugging purpose with comments and ensure the behavior remains consistent across modules (e.g., acquisition/localization loops such as src/odemis/acq/align/z_localization.py).

Applied to files:

• src/odemis/acq/move.py
• src/odemis/gui/comp/slider.py

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: build (ubuntu-22.04)
• GitHub Check: build (ubuntu-24.04)

🔇 Additional comments (1)

src/odemis/acq/move.py (1)

581-584: LGTM! Rotation correction is now applied conditionally.

The guard prevents unnecessary metadata updates when scan rotation is zero. The comment clearly explains the purpose.

Note: There's a slight behavioral difference with MeteorTescan1PostureManager._update_conversion() (line 1677), which calls _set_scanner_rotation_cor(sr) unconditionally. If the Tescan behavior should also be conditional, consider aligning them for consistency.