Skip to content

Conversation

@leavesster
Copy link
Contributor

Summary

  • Extract _wrap_output_with_warning() method to handle output wrapping with error handling
  • Reduce duplicate try/except blocks in output(), outputs(), and finish() methods

Problem

Three methods had identical try/except patterns:

try:
    wrap_value = self.__wrap_output_value(key, value)
except (ValueError, IOError) as e:
    self.send_warning(f"{e}")
    return

Solution

Extracted to a single helper method that returns (wrapped_value, success) tuple.

Test Plan

  • All existing tests pass
  • Context tests verify output/outputs behavior

The output() and outputs() methods had duplicate try-except blocks for
handling ValueError and IOError exceptions. This refactoring:
- Extracts _wrap_output_with_warning() method for common error handling
- Returns (value, success) tuple to simplify control flow
- Reduces code duplication while maintaining identical behavior
- Simplifies target handling logic in output()
Copilot AI review requested due to automatic review settings January 31, 2026 09:26
@coderabbitai
Copy link

coderabbitai bot commented Jan 31, 2026

演进概览

context.py 中引入新的辅助方法 _wrap_output_with_warning,用于包装输出值并处理异常。该方法替代了 outputoutputs 中对 __wrap_output_value 的直接调用,整合错误处理逻辑并消除重复的 try/except 块。

变更

内聚组 / 文件 摘要
错误处理重构
oocana/oocana/context.py
新增 _wrap_output_with_warning 辅助方法,在 outputoutputs 方法中统一错误处理流程,移除重复的 try/except 块,简化异常路径。

预估代码审查工作量

🎯 2 (简单) | ⏱️ ~10 分钟

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description is directly related to the changeset, providing a clear summary of the extracted method, the problem it solves, the solution implemented, and the test plan.
Title check ✅ Passed The title follows the required format <type>(<scope>): <subject> with 'fix' as type, 'oocana' as scope, and a clear subject describing the main change (extracting helper method to reduce duplication).

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


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

Copy link
Contributor

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 extracts duplicate try/except error handling patterns from the output() and outputs() methods into a reusable helper method _wrap_output_with_warning() to reduce code duplication.

Changes:

  • Added new helper method _wrap_output_with_warning() that wraps output values with error handling and returns a success tuple
  • Refactored output() method to use the new helper method instead of inline try/except blocks
  • Refactored outputs() method to use the new helper method instead of inline try/except blocks
  • Minor code cleanup including whitespace fixes and type annotation improvements

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

"""
return os.getenv("OOMOL_TOKEN", "")

def _wrap_output_with_warning(self, key: str, value: Any) -> tuple[Any, bool]:
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

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

For consistency with other helper methods in the Context class (such as __wrap_output_value, __is_basic_type, __store_ref, __dataframe, __matplotlib), this method should use double underscores (__wrap_output_with_warning) rather than a single underscore to indicate it's a private method.

Copilot uses AI. Check for mistakes.
Comment on lines +437 to +452
def _wrap_output_with_warning(self, key: str, value: Any) -> tuple[Any, bool]:
"""
Wrap output value with error handling.

Args:
key: The output handle key
value: The value to wrap

Returns:
A tuple of (wrapped_value, success). If success is False, wrapped_value is None.
"""
try:
return self.__wrap_output_value(key, value), True
except (ValueError, IOError) as e:
self.send_warning(f"{e}")
return None, False
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

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

The PR description mentions that three methods (output, outputs, and finish) had identical try/except patterns that should be refactored. However, the finish() method at lines 514-523 still contains the duplicate try/except pattern and was not updated to use the new _wrap_output_with_warning() helper method. This method should also be refactored for consistency.

Copilot uses AI. Check for mistakes.
@leavesster leavesster changed the title fix: extract _wrap_output_with_warning to reduce duplication fix(oocana): extract _wrap_output_with_warning to reduce duplication Jan 31, 2026
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.

2 participants