[feat] Introduce MODEL_OPTIONS variable to odemis.conf#3412
[feat] Introduce MODEL_OPTIONS variable to odemis.conf#3412pieleric wants to merge 1 commit intodelmic:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for a new MODEL_OPTIONS entry in odemis.conf to enable interactive microscope model selection by expanding one or more glob patterns and delegating selection to odemis-select-mic-start.
Changes:
- Add
run_model_options()to expandMODEL_OPTIONSglob patterns and callodemis-select-mic-startwith the matched files. - Update
odemis-startstartup flow to invokeMODEL_OPTIONSselection when no model is provided on the command line. - Document
MODEL_OPTIONSusage in the shippedinstall/linux/etc/odemis.conftemplate.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/odemis/odemisd/start.py |
Adds the MODEL_OPTIONS expansion/launcher and integrates it into main() startup logic. |
install/linux/etc/odemis.conf |
Documents the new MODEL_OPTIONS configuration option and intended usage. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis pull request adds support for interactive model file selection during startup via a new Sequence DiagramsequenceDiagram
participant Main as Startup (main)
participant Config as Configuration
participant Glob as Glob Expansion
participant Select as odemis-select-mic-start
participant GUI as User GUI
Main->>Config: Read odemis.conf
Config-->>Main: MODEL_OPTIONS present?
alt MODEL_OPTIONS configured
Main->>Glob: Expand pattern_str via glob.glob
Glob-->>Main: List of .odm.yaml files
alt Files found
Main->>Select: Call with matched files
Select->>GUI: Display model selection dialog
GUI-->>Select: User selection
Select-->>Main: Selected model path
else No files matched
Main->>GUI: Show error box
GUI-->>Main: Error acknowledged
end
else MODEL_OPTIONS not set
Main->>Main: Proceed to normal startup flow
end
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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.
🧹 Nitpick comments (2)
src/odemis/odemisd/start.py (2)
668-668: Consider using list unpacking for clarity.The static analyzer suggests using unpacking syntax instead of list concatenation.
Suggested change
- return subprocess.call(["odemis-select-mic-start"] + files) + return subprocess.call(["odemis-select-mic-start", *files])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/odemis/odemisd/start.py` at line 668, Replace the list concatenation used in the subprocess.call invocation with Python list unpacking for clarity: locate the subprocess.call([...]) call that currently builds the argv with ["odemis-select-mic-start"] + files and change it to use unpacking so the command list is created as ["odemis-select-mic-start", *files]; update the subprocess.call invocation (the call that invokes "odemis-select-mic-start") accordingly to pass the unpacked list.
655-658: Consider deduplicating matched files.If overlapping glob patterns are provided (e.g.,
*.yaml *-sim.yaml), the same file could appear multiple times in the selection dialog.Suggested change to deduplicate and sort globally
patterns = shlex.split(pattern_str) - files = [] + files = set() for pattern in patterns: - files.extend(sorted(glob.glob(pattern))) + files.update(glob.glob(pattern)) + files = sorted(files)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/odemis/odemisd/start.py` around lines 655 - 658, The file-matching loop can produce duplicates when patterns overlap: after building patterns from pattern_str and extending files in the loop (variables: pattern_str, patterns, files), deduplicate the collected filenames and then sort them globally before returning/using them; e.g., replace the current files handling with a step that removes duplicates (e.g., by converting to a set or using an order-preserving dedupe) and then apply sorted(...) so the selection dialog shows each file only once and in deterministic order.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/odemis/odemisd/start.py`:
- Line 668: Replace the list concatenation used in the subprocess.call
invocation with Python list unpacking for clarity: locate the
subprocess.call([...]) call that currently builds the argv with
["odemis-select-mic-start"] + files and change it to use unpacking so the
command list is created as ["odemis-select-mic-start", *files]; update the
subprocess.call invocation (the call that invokes "odemis-select-mic-start")
accordingly to pass the unpacked list.
- Around line 655-658: The file-matching loop can produce duplicates when
patterns overlap: after building patterns from pattern_str and extending files
in the loop (variables: pattern_str, patterns, files), deduplicate the collected
filenames and then sort them globally before returning/using them; e.g., replace
the current files handling with a step that removes duplicates (e.g., by
converting to a set or using an order-preserving dedupe) and then apply
sorted(...) so the selection dialog shows each file only once and in
deterministic order.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1020000f-12ba-48df-b0a6-46786aa3cab4
📒 Files selected for processing (2)
install/linux/etc/odemis.confsrc/odemis/odemisd/start.py
This variable should contain a filename pattern of microscope files. If it's defined, the pattern is used to pass a series of microscope files to odemis-select-mic-start, which eventually will call odemis-start with a specific microscope file. This allows to use odemis-select-mic-start without a special launcher. Instead, just an extra line on odemis.conf is sufficient.
pieleric
force-pushed
the
feat-introduce-model_options-variable-to-odemis-conf
branch
from
2def123 to
03bd016
Compare
This variable should contain a filename pattern of microscope files.
If it's defined, the pattern is used to pass a series of microscope
files to odemis-select-mic-start, which eventually will call
odemis-start with a specific microscope file.
This allows to use odemis-select-mic-start without a special launcher.
Instead, just an extra line on odemis.conf is sufficient.