Skip to content

[fix] add additional sp_options arg to FastEMStreamController needed for base class StreamController#3422

Merged
pieleric merged 1 commit intodelmic:masterfrom
nandishjpatel:fix-fastem-stream-controller
Mar 27, 2026
Merged

[fix] add additional sp_options arg to FastEMStreamController needed for base class StreamController#3422
pieleric merged 1 commit intodelmic:masterfrom
nandishjpatel:fix-fastem-stream-controller

Conversation

@nandishjpatel
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown

Copilot AI left a comment

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 updates FastEMStreamController to accept and forward the sp_options argument so it remains compatible with the base StreamController initialization contract used by stream-bar creation code.

Changes:

  • Add sp_options parameter to FastEMStreamController.__init__.
  • Forward sp_options through to StreamController.__init__ via super().__init__(..., sp_options=sp_options).

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 26, 2026

📝 Walkthrough

Walkthrough

The FastEMStreamController.__init__ method signature in src/odemis/gui/cont/stream.py has been modified to accept an additional optional parameter sp_options. This parameter is forwarded to the parent class StreamController constructor, which already supports it. The parameter controls the option bitmask used during StreamPanel initialization.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive No description was provided by the author, making it impossible to assess whether the description is related to the changeset. Add a brief description explaining why the sp_options argument is needed and how it benefits the base class integration.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding the sp_options argument to FastEMStreamController to support the base class StreamController.

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

Copy link
Copy Markdown

@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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/odemis/gui/cont/stream.py`:
- Around line 1384-1389: Add parameter and return type annotations to
FastEMStreamController.__init__: annotate stream_bar, stream, tab_data_model
with their concrete types used elsewhere (e.g., StreamBar, Stream,
TabDataModel), annotate show_panel as bool, view and sb_ctrl as Optional[...],
and sp_options as Optional[dict] (or Optional[Mapping[str, Any]]), and set the
return type to None. Also add a concise docstring to __init__ describing the
purpose of the constructor and each parameter (including defaults and
optionality) and any side effects (e.g., registering with stream_bar), matching
the project's docstring style.
🪄 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: 099ee25f-7704-4047-9c69-79e5d200c77d

📥 Commits

Reviewing files that changed from the base of the PR and between f9f275c and 3323c88.

📒 Files selected for processing (1)
  • src/odemis/gui/cont/stream.py

Comment on lines 1384 to 1389
def __init__(self, stream_bar, stream, tab_data_model, show_panel=True, view=None,
sb_ctrl=None):
sb_ctrl=None, sp_options=None):
super().__init__(
stream_bar=stream_bar, stream=stream, tab_data_model=tab_data_model,
show_panel=show_panel, view=view, sb_ctrl=sb_ctrl
show_panel=show_panel, view=view, sb_ctrl=sb_ctrl, sp_options=sp_options
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether __init__ in FastEMStreamController has full annotations + docstring.
python - <<'PY'
import ast, pathlib

path = pathlib.Path("src/odemis/gui/cont/stream.py")
mod = ast.parse(path.read_text(encoding="utf-8"))

for node in mod.body:
    if isinstance(node, ast.ClassDef) and node.name == "FastEMStreamController":
        for fn in node.body:
            if isinstance(fn, ast.FunctionDef) and fn.name == "__init__":
                missing_args = [
                    a.arg for a in fn.args.args
                    if a.arg != "self" and a.annotation is None
                ]
                missing_ret = fn.returns is None
                missing_doc = ast.get_docstring(fn) is None
                print("missing_arg_annotations:", missing_args)
                print("missing_return_annotation:", missing_ret)
                print("missing_docstring:", missing_doc)
PY

Repository: delmic/odemis

Length of output: 227


Add type hints and a docstring to FastEMStreamController.__init__.

The constructor is missing all parameter and return type annotations, plus a docstring. These are required by the repository's coding guidelines for Python files.

Proposed patch
+from typing import Any
...
 class FastEMStreamController(StreamController):
-    def __init__(self, stream_bar, stream, tab_data_model, show_panel=True, view=None,
-                 sb_ctrl=None, sp_options=None):
+    def __init__(
+        self,
+        stream_bar: Any,
+        stream: acqstream.SEMStream,
+        tab_data_model: Any,
+        show_panel: bool = True,
+        view: Any = None,
+        sb_ctrl: Any = None,
+        sp_options: int | None = None,
+    ) -> None:
+        """
+        Initialize the FastEM stream controller.
+
+        :param stream_bar: Container for stream panels.
+        :param stream: Stream handled by this controller.
+        :param tab_data_model: Tab data model.
+        :param show_panel: Whether to show the panel immediately.
+        :param view: Linked view for visibility handling.
+        :param sb_ctrl: Parent stream bar controller.
+        :param sp_options: StreamPanel option bitmask.
+        """
         super().__init__(
             stream_bar=stream_bar, stream=stream, tab_data_model=tab_data_model,
             show_panel=show_panel, view=view, sb_ctrl=sb_ctrl, sp_options=sp_options
         )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/odemis/gui/cont/stream.py` around lines 1384 - 1389, Add parameter and
return type annotations to FastEMStreamController.__init__: annotate stream_bar,
stream, tab_data_model with their concrete types used elsewhere (e.g.,
StreamBar, Stream, TabDataModel), annotate show_panel as bool, view and sb_ctrl
as Optional[...], and sp_options as Optional[dict] (or Optional[Mapping[str,
Any]]), and set the return type to None. Also add a concise docstring to
__init__ describing the purpose of the constructor and each parameter (including
defaults and optionality) and any side effects (e.g., registering with
stream_bar), matching the project's docstring style.

@pieleric pieleric merged commit 95c871e into delmic:master Mar 27, 2026
9 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.

4 participants