fix: typing

[johanneskoester]

Summary by CodeRabbit

• Refactor
  • Updated log handler settings to support optional values, allowing implementations to omit settings when not required.

[coderabbitai]

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)

• pyproject.toml is excluded by !pyproject.toml

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The return type of the get_log_handler_settings() method in the TestLogHandlerBase abstract class is widened from LogHandlerSettingsBase to Optional[LogHandlerSettingsBase], permitting implementations to return None in addition to handler settings objects.

Changes

Cohort / File(s)	Summary
Test Handler Base Return Type src/snakemake_interface_logger_plugins/tests.py	Updated get_log_handler_settings() method return type annotation from LogHandlerSettingsBase to Optional[LogHandlerSettingsBase] in TestLogHandlerBase class, allowing None as a valid return value.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• feat: Add test baseclass #39: Directly modifies the same abstract method get_log_handler_settings in TestLogHandlerBase to change its return type signature.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'fix: typing' is vague and generic, using non-descriptive language that doesn't convey the specific nature of the type change being made.	Make the title more specific, such as 'fix: allow Optional return type for get_log_handler_settings' to clearly describe what typing issue was fixed.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
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 unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/types

---

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]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/snakemake_interface_logger_plugins/tests.py (1)

7-7: ⚠️ Potential issue | 🔴 Critical

Optional is not imported — this will raise a NameError at module import time.

Line 80 uses Optional[LogHandlerSettingsBase] as a return type annotation, but Optional is never imported. Because there is no from __future__ import annotations, Python evaluates annotations eagerly, so the class definition will fail with NameError: name 'Optional' is not defined the moment the module is imported.

🐛 Proposed fix

-from typing import Type
+from typing import Optional, Type

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/snakemake_interface_logger_plugins/tests.py` at line 7, The module uses
Optional in a type annotation (Optional[LogHandlerSettingsBase]) but never
imports it, causing a NameError at import; fix by importing Optional from typing
(add Optional to the existing "from typing import Type" import) or alternatively
enable postponed evaluation of annotations with "from __future__ import
annotations"; ensure the reference to Optional is resolved where
LogHandlerSettingsBase is used so the annotation no longer raises at module
import.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/snakemake_interface_logger_plugins/tests.py`:
- Line 7: The module uses Optional in a type annotation
(Optional[LogHandlerSettingsBase]) but never imports it, causing a NameError at
import; fix by importing Optional from typing (add Optional to the existing
"from typing import Type" import) or alternatively enable postponed evaluation
of annotations with "from __future__ import annotations"; ensure the reference
to Optional is resolved where LogHandlerSettingsBase is used so the annotation
no longer raises at module import.