Skip to content

Conversation

@K4rishma
Copy link
Contributor

No description provided.

@coderabbitai
Copy link

coderabbitai bot commented Jan 19, 2026

📝 Walkthrough

Walkthrough

This pull request adds support for a refractive index mismatch correction metadata key and uses it in correlation Z-corrections. A new metadata constant MD_RIM_COR was added. Two simulation Optical Objective YAML files gained RIM_COR: 0.495 in their metadata. The CorrelationPointsController now reads the lens metadata into an instance attribute refractive_scale (falling back to a default 0.495) and uses it for Z-correction instead of the previous module-level constant.

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided by the author, so there is no content to evaluate. Add a pull request description explaining the motivation, implementation details, and any testing performed for this metadata addition.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding lens metadata (MD_RIM_COR) to enable dynamic configuration of the refractive index mismatch correction factor.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ 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

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/odemis/gui/cont/multi_point_correlation.py`:
- Around line 171-172: The code assumes self._main_data_model.lens exists before
calling lens.getMetadata(), which can be None; update the block around lens_md
and self.refractive_scale to defensively handle a missing lens by first checking
self._main_data_model.lens (or using a conditional expression) so lens_md falls
back to an empty dict when lens is None, then call .get(model.MD_RIM_COR,
REFRACTIVE_SCALE) to assign self.refractive_scale; target the code that uses
self._main_data_model.lens.getMetadata(), self.refractive_scale,
model.MD_RIM_COR and REFRACTIVE_SCALE to implement this null-safe behavior.
🧹 Nitpick comments (2)
install/linux/usr/share/odemis/sim/meteor-fibsem-sim.odm.yaml (1)

118-121: Minor formatting inconsistency and clarify comment.

  1. There's a space before the colon in RIM_COR : (line 120) which is inconsistent with other YAML entries in this file (e.g., na: 0.85, ri: 1.0).

  2. The comment references "50x lens" and "100x0.9 na" values, but the lens magnification in the init block is 84.0. Consider updating the comment to clarify which value applies to this 84x configuration.

Suggested fix
     metadata: {
         # Refractive index mismatch correction factor
-        RIM_COR : 0.495,  # generally for 50x lens use 0.495, 100x0.9 na use 0.6
+        RIM_COR: 0.495,  # for this 84x lens configuration
     },
install/linux/usr/share/odemis/sim/meteor-tfs3-sim.odm.yaml (1)

73-76: Same formatting and comment issues as the other YAML file.

Apply the same fix for consistency: remove the space before the colon and clarify the comment for the 84x lens configuration.

Suggested fix
     metadata: {
         # Refractive index mismatch correction factor
-        RIM_COR : 0.495,  # generally for 50x lens use 0.495, 100x0.9 na use 0.6
+        RIM_COR: 0.495,  # for this 84x lens configuration
     },

For Refractive Index mismatch correction
@K4rishma K4rishma force-pushed the refractive_index_metadata branch from a70ef1b to ea01edf Compare January 27, 2026 08:03
@K4rishma K4rishma requested a review from pieleric January 27, 2026 08:04
@pieleric pieleric merged commit e3741e5 into delmic:master Jan 28, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants