feat: paddleocr v6 - #2320
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough此 PR 为 onnxocr 模块添加 PP-OCRv6 模型支持,创建统一的 ONNX 推理会话管理层,将日志系统与框架集成,为各预测模块添加 GPU 设备管理与预热机制,并优化内存操作与错误处理流程。 ChangesPP-OCRv6 支持与推理引擎重构
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
src/onnxocr/inference_engine.py (1)
156-156: 📐 Maintainability & Code Quality | ⚡ Quick win路径检查建议改为
pathlib.Path。Line 156 使用
os.path.exists,建议统一为Path(model_path).exists(),与项目路径处理约定保持一致并提升可读性。As per coding guidelines,
**/*.py明确“Preferpathliblibrary for path handling instead ofos.path”。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/onnxocr/inference_engine.py` at line 156, Replace the os.path.exists check with pathlib: change the conditional that tests model_path (the "if not os.path.exists(model_path)" check) to use Path(model_path).exists() (or if model_path may already be a Path, call model_path.exists()), and add/import "from pathlib import Path" at the top of the module so path handling follows the project's pathlib convention.Source: Coding guidelines
src/onnxocr/onnx_paddleocr.py (1)
92-92: 🎯 Functional Correctness日志占位符
{}与现有 logger shim 匹配,无需改为 f-string
src/onnxocr/onnx_paddleocr.py 第 92 行的log.info("... cls={}, ...", self.use_angle_cls)会被src/onnxocr/logger.py的_LoggerShim通过str(msg).format(*args, **kwargs)渲染,因此不构成真实风格/兼容性问题;仓库内也多处已使用相同{}风格。若仍触发 PLE1205,优先调整对应 lint 规则/配置,而不是在此处改写。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/onnxocr/onnx_paddleocr.py` at line 92, Keep the log call in onnx_paddleocr.py as log.info("OCR model initialized: det=True, cls={}, rec=True", self.use_angle_cls) because src/onnxocr/logger.py::_LoggerShim expects format-style placeholders; revert any change to an f-string and instead update your lint configuration to suppress PLE1205 for this pattern (e.g., disable PLE1205 globally or for this module, or add a logger-specific exemption in pyproject/.pylintrc) so the logger shim formatting is allowed without changing code.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/onnxocr/inference_engine.py`:
- Line 8: Replace relative imports inside the onnxocr package with absolute
module imports: e.g., change “from .logger import get_logger” to “from
onnxocr.logger import get_logger” (and similarly for other symbols). Apply this
consistently across the listed modules (inference_engine.py, predict_base.py,
operators.py, predict_system.py, predict_det.py, predict_cls.py, predict_rec.py,
onnx_paddleocr.py) and any other files using “from .” or “from ..”; ensure
package import paths use the absolute onnxocr.<module> form and that package
__init__.py exists so absolute imports resolve.
- Line 4: Replace typing generics with Python 3.11 built-ins and use | for
unions throughout src/onnxocr/inference_engine.py: remove
List/Dict/Optional/Sequence/Union imports from typing and update all annotations
(notably the Provider type alias/definition and all function/method signatures
between the Provider definition and lines ~37–155) to use list[...], dict[...],
X | None and A | B forms (and Tuple -> tuple[...] where present); ensure any
remaining typing-only names are either dropped or imported from collections.abc
if needed (e.g., Callable/Iterable) and adjust return/parameter annotations
accordingly to keep semantics identical.
In `@src/onnxocr/logger.py`:
- Around line 42-43: The function add_file_sink currently has a silent pass
which causes callers to believe file logging is enabled; change it to explicitly
raise NotImplementedError (or implement the file-sink behavior) so callers get
immediate, clear feedback. Locate the add_file_sink(path: str, level: str =
"DEBUG", rotation: str = "10 MB") definition and replace the pass with: raise
NotImplementedError("add_file_sink is not implemented") (or implement the
intended file-logging behavior and ensure it logs/returns consistently with
other logger helpers).
- Around line 11-43: Add missing type annotations: annotate class attribute
self.name as str in _LoggerShim.__init__(name: str) -> None; change
_format(self, msg: str) -> str; annotate info/debug/warning/error as (self, msg:
str, *args: Any, **kwargs: Any) -> None (and import Any from typing if not
present); annotate get_logger(name: str = "OnnxOCR") -> _LoggerShim and
add_file_sink(path: str, level: str = "DEBUG", rotation: str = "10 MB") -> None.
Ensure any added typing imports are included at the top of the file.
In `@src/onnxocr/onnx_paddleocr.py`:
- Around line 43-49: The PP-OCRv6 defaults are being applied when only
ocr_model_size is present (because to_dict always supplies a default size); to
fix, change _build_ppocrv6_defaults to require an explicit ocr_model_name before
treating the model as v6—i.e., read/pop ocr_model_name first and if it's falsy
return {} immediately instead of calling _normalize_ppocrv6_size with only
model_size; do the same defensive early-return in the related helper blocks
mentioned (the other PP-OCRv6-default builders around the regions that call
_normalize_ppocrv6_size) so that defaults like det_db_box_thresh are only
injected when ocr_model_name indicates a v6 model. Ensure you reference and
update uses of _normalize_ppocrv6_size, _build_ppocrv6_defaults and the other
similar builder functions to implement the early-return on missing
ocr_model_name.
In `@src/onnxocr/predict_base.py`:
- Around line 11-12: Add explicit type annotations to get_onnx_session: annotate
parameters model_dir: str, use_gpu: bool, gpu_id: int = 0 and the return type to
match create_session -> InferenceSession; ensure you import or reference
InferenceSession from the onnxruntime (or the module that defines
create_session) so the signature reads get_onnx_session(model_dir: str, use_gpu:
bool, gpu_id: int = 0) -> InferenceSession and returns create_session(model_dir,
use_gpu=use_gpu, gpu_id=gpu_id).
In `@src/onnxocr/predict_system.py`:
- Around line 3-7: Update the relative imports in predict_system.py (the imports
of predict_det, predict_cls, predict_rec, logger.get_logger, and utils functions
get_rotate_crop_image/get_minarea_rect_crop) to absolute imports (e.g., from
onnxocr import predict_det / from onnxocr.logger import get_logger / from
onnxocr.utils import ...) and apply the same change across onnx_paddleocr.py,
predict_cls.py, predict_det.py and predict_rec.py; keep using relative/type-only
imports only inside TYPE_CHECKING blocks if needed for typing to avoid runtime
import issues.
---
Nitpick comments:
In `@src/onnxocr/inference_engine.py`:
- Line 156: Replace the os.path.exists check with pathlib: change the
conditional that tests model_path (the "if not os.path.exists(model_path)"
check) to use Path(model_path).exists() (or if model_path may already be a Path,
call model_path.exists()), and add/import "from pathlib import Path" at the top
of the module so path handling follows the project's pathlib convention.
In `@src/onnxocr/onnx_paddleocr.py`:
- Line 92: Keep the log call in onnx_paddleocr.py as log.info("OCR model
initialized: det=True, cls={}, rec=True", self.use_angle_cls) because
src/onnxocr/logger.py::_LoggerShim expects format-style placeholders; revert any
change to an f-string and instead update your lint configuration to suppress
PLE1205 for this pattern (e.g., disable PLE1205 globally or for this module, or
add a logger-specific exemption in pyproject/.pylintrc) so the logger shim
formatting is allowed without changing code.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2b838d48-96b6-4016-b413-73b5a83eaffb
📒 Files selected for processing (13)
src/one_dragon/base/config/basic_model_config.pysrc/one_dragon/base/matcher/ocr/onnx_ocr_matcher.pysrc/onnxocr/inference_engine.pysrc/onnxocr/logger.pysrc/onnxocr/onnx_paddleocr.pysrc/onnxocr/operators.pysrc/onnxocr/predict_base.pysrc/onnxocr/predict_cls.pysrc/onnxocr/predict_det.pysrc/onnxocr/predict_rec.pysrc/onnxocr/predict_system.pysrc/onnxocr/readme.mdsrc/onnxocr/utils.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/onnxocr/predict_system.py (1)
67-67: 💤 Low value
strict=False正确处理了角度分类器可能过滤裁剪图的情况,但建议增加长度不匹配时的日志记录。当角度分类器启用时(第 58-59 行),
img_crop_list的长度可能因过滤而减少,导致dt_boxes和rec_res长度不一致。使用strict=False可以正确处理这种情况,与onnx_paddleocr.py的模式一致。建议在长度不匹配时记录警告日志,帮助调试和监控:
💡 建议的增强
+ if len(dt_boxes) != len(rec_res): + log.warning( + "检测框数量({})与识别结果数量({})不匹配,将截断至较短列表", + len(dt_boxes), len(rec_res) + ) for box, rec_result in zip(dt_boxes, rec_res, strict=False):🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/onnxocr/predict_system.py` at line 67, Before the loop "for box, rec_result in zip(dt_boxes, rec_res, strict=False):" add a length check and emit a warning when lengths differ (e.g., if len(dt_boxes) != len(rec_res)): log the counts of dt_boxes, rec_res and img_crop_list plus whether the angle classifier flag is enabled so operators can triage filtering issues; use the module's existing logger instance (or self.logger if inside a class) and keep the message concise and actionable; do not change the zip behavior (keep strict=False) — only add the pre-loop warning and continue as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/onnxocr/predict_system.py`:
- Line 67: Before the loop "for box, rec_result in zip(dt_boxes, rec_res,
strict=False):" add a length check and emit a warning when lengths differ (e.g.,
if len(dt_boxes) != len(rec_res)): log the counts of dt_boxes, rec_res and
img_crop_list plus whether the angle classifier flag is enabled so operators can
triage filtering issues; use the module's existing logger instance (or
self.logger if inside a class) and keep the message concise and actionable; do
not change the zip behavior (keep strict=False) — only add the pre-loop warning
and continue as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 425a5358-a8d9-4f90-a29a-1563700c3450
📒 Files selected for processing (9)
src/onnxocr/inference_engine.pysrc/onnxocr/logger.pysrc/onnxocr/onnx_paddleocr.pysrc/onnxocr/operators.pysrc/onnxocr/predict_base.pysrc/onnxocr/predict_cls.pysrc/onnxocr/predict_det.pysrc/onnxocr/predict_rec.pysrc/onnxocr/predict_system.py
🚧 Files skipped from review as they are similar to previous changes (6)
- src/onnxocr/logger.py
- src/onnxocr/predict_base.py
- src/onnxocr/inference_engine.py
- src/onnxocr/onnx_paddleocr.py
- src/onnxocr/operators.py
- src/onnxocr/predict_cls.py
| dict_name = 'ppocrv6_dict.txt' if 'ppocrv6' in ocr_model_name else 'ppocrv5_dict.txt' | ||
| return [ | ||
| os.path.join(base_dir, 'det.onnx'), | ||
| os.path.join(base_dir, 'rec.onnx'), | ||
| os.path.join(base_dir, 'cls.onnx'), | ||
| os.path.join(base_dir, 'ppocrv5_dict.txt'), | ||
| os.path.join(base_dir, dict_name), |
|
|
||
| def get_ocr_opts() -> list[ConfigItem]: | ||
| models_list = [DEFAULT_OCR_MODEL_NAME] | ||
| models_list = [DEFAULT_OCR_MODEL_NAME, 'ppocrv6_small'] |
There was a problem hiding this comment.
等测试通过了后,把v5直接换成为v6
f3f66ca to
3b9d2d5
Compare
3b9d2d5 to
60dbfd4
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/onnxocr/onnx_paddleocr.py (1)
97-134:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win函数内再次导入
log会导致UnboundLocalError。在
ocr中,Line 133 的from one_dragon.utils.log_utils import log会把log绑定为函数局部变量;因此 Line 99 的log.warning(...)会在运行时触发“引用前赋值”错误。这个路径在默认cls=True且use_angle_cls=False时可直接命中。请移除函数内log导入并复用模块级 logger。💡 建议修改
- except Exception: - from one_dragon.utils.log_utils import log - log.error('OCR推理出错', exc_info=True) + except Exception: + log.error("OCR推理出错", exc_info=True) try: from one_dragon.utils import debug_utils debug_image = img[0] if isinstance(img, list) else img - debug_utils.save_debug_image(image=debug_image, prefix='ocr_error') + debug_utils.save_debug_image(image=debug_image, prefix="ocr_error") except Exception: - log.warning('保存OCR错误调试图片失败', exc_info=True) + log.warning("保存OCR错误调试图片失败", exc_info=True) raise🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/onnxocr/onnx_paddleocr.py` around lines 97 - 134, The function ocr has a local import of log at line 133 which creates a scope binding issue. When Python sees that log is assigned locally within the function (via the from...import statement in the except block), it treats all references to log in the entire function as local variable references, including the log.warning call at line 99 which appears before the import. This causes an UnboundLocalError at runtime. Remove the local import statement from the except block at line 133 and instead rely on the module-level log import that should already exist at the top of the file. The log.error call in the except block will then correctly reference the module-level logger.
🧹 Nitpick comments (4)
src/onnxocr/onnx_paddleocr.py (1)
28-49: ⚡ Quick win新增函数签名建议补齐类型注解。
_normalize_ppocrv6_size、_build_ppocrv6_defaults与ocr的参数当前是未注解状态,和src/**/*.py的类型约定不一致。建议至少为参数与返回值补上明确类型。
As per coding guidelines:src/**/*.py要求“所有函数签名和类成员变量必须有类型注解”,并使用list[str]/X | Y风格。Also applies to: 97-97
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/onnxocr/onnx_paddleocr.py` around lines 28 - 49, Add type annotations to the function signatures for _normalize_ppocrv6_size and _build_ppocrv6_defaults (and the function referenced at line 97) to comply with the project's coding guidelines. For _normalize_ppocrv6_size, annotate the model_name and model_size parameters with appropriate types (likely Optional[str] or str | None) and the return type (likely str | None). For _build_ppocrv6_defaults, annotate the kwargs parameter (likely dict or similar) and the return type (likely dict). Use modern Python type annotation style consistent with the rest of the codebase, using union syntax like X | Y instead of Union[X, Y].Source: Coding guidelines
src/onnxocr/operators.py (1)
16-16: ⚡ Quick win本次改动触达的方法签名建议补齐类型注解。
NormalizeImage.__init__、DetResizeForTest.__init__、KeepKeys.__init__/__call__在变更后仍为无注解签名,建议按 Python 3.11 风格补齐参数与返回类型。
As per coding guidelines:src/**/*.py要求“所有函数签名和类成员变量必须有类型注解”。Also applies to: 43-43, 188-191
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/onnxocr/operators.py` at line 16, Add complete type annotations to the method signatures as per Python 3.11 style. For NormalizeImage.__init__ at line 16, DetResizeForTest.__init__ at line 43, and KeepKeys.__init__ and KeepKeys.__call__ at lines 188-191, add type hints for all parameters (scale, mean, std, order, kwargs, etc.) and return type annotations. Ensure each parameter has its corresponding type hint and include the return type (typically None for __init__ and __call__ methods) following the format of parameter_name: type -> return_type.Source: Coding guidelines
src/onnxocr/inference_engine.py (2)
157-161: ⚡ Quick win路径检查建议改为
pathlib.Path。Line 157 仍使用
os.path.exists,建议统一为Path(model_path).exists(),以贴合仓库的路径处理约定。💡 建议修改
-import os +from pathlib import Path ... - if not os.path.exists(model_path): + if not Path(model_path).exists():As per coding guidelines:
**/*.py与src/**/*.py均要求优先使用pathlib处理路径。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/onnxocr/inference_engine.py` around lines 157 - 161, Replace the `os.path.exists()` call in the model_path existence check with `Path(model_path).exists()` to align with the repository's pathlib conventions. Import `Path` from `pathlib` at the top of the file if not already imported, then update the condition that raises FileNotFoundError to use `Path(model_path).exists()` instead of `os.path.exists(model_path)` for consistency with the codebase's path handling standards.Source: Coding guidelines
97-99: ⚡ Quick win
ProviderConfig的成员变量建议补齐类型注解。
self.engine_cfg在 Line 98 被赋值,但类内未声明成员类型,和当前src/**/*.py约定不一致。建议在类体内显式声明,例如engine_cfg: Any。
As per coding guidelines:src/**/*.py要求“所有函数签名和类成员变量必须有类型注解”。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/onnxocr/inference_engine.py` around lines 97 - 99, The class member variable `engine_cfg` lacks a type annotation at the class level in the `ProviderConfig` class, which violates the coding convention requiring all class member variables to have explicit type annotations. Add a class-level type annotation for `engine_cfg` (with type `Any`) in the class body before the `__init__` method to match the existing code style in `src/**/*.py`.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/one_dragon/base/config/basic_model_config.py`:
- Around line 22-23: The ocr_use_gpu method only checks for the new
configuration key 'ocr_use_gpu' and lacks a fallback mechanism for the legacy
key 'ocr_gpu', which causes existing user configurations to silently revert to
False during upgrades. Modify the ocr_use_gpu method to first attempt retrieving
the value from the new key 'ocr_use_gpu', then fall back to the old key
'ocr_gpu' if the new key is not found, and only default to False if neither key
exists in the configuration.
In `@src/one_dragon/base/operation/one_dragon_context.py`:
- Around line 487-497: The current code assigns the OnnxOcrMatcher instance to
ocr_service.ocr_matcher and cv_service.ocr before calling init_model, which
means if init_model fails and returns False, the global references will hold an
uninitialized matcher causing subsequent OCR operations to fail. Move the
assignments of self.ocr_service.ocr_matcher and self.cv_service.ocr (currently
after self.ocr.overlay_debug_bus assignment) to execute only after init_model is
called and succeeds, so that global references only get a fully initialized and
ready OCR matcher.
In `@src/onnxocr/predict_cls.py`:
- Line 22: The log.info call uses a placeholder-based string formatting style
that triggers a static check error (PLE1205) and violates the repository's
coding guidelines. Convert the log message to use f-string formatting instead by
replacing the placeholder string with an f-string and removing the separate
argument, so that the message string with the embedded variable reference is
passed directly to the log.info method.
---
Outside diff comments:
In `@src/onnxocr/onnx_paddleocr.py`:
- Around line 97-134: The function ocr has a local import of log at line 133
which creates a scope binding issue. When Python sees that log is assigned
locally within the function (via the from...import statement in the except
block), it treats all references to log in the entire function as local variable
references, including the log.warning call at line 99 which appears before the
import. This causes an UnboundLocalError at runtime. Remove the local import
statement from the except block at line 133 and instead rely on the module-level
log import that should already exist at the top of the file. The log.error call
in the except block will then correctly reference the module-level logger.
---
Nitpick comments:
In `@src/onnxocr/inference_engine.py`:
- Around line 157-161: Replace the `os.path.exists()` call in the model_path
existence check with `Path(model_path).exists()` to align with the repository's
pathlib conventions. Import `Path` from `pathlib` at the top of the file if not
already imported, then update the condition that raises FileNotFoundError to use
`Path(model_path).exists()` instead of `os.path.exists(model_path)` for
consistency with the codebase's path handling standards.
- Around line 97-99: The class member variable `engine_cfg` lacks a type
annotation at the class level in the `ProviderConfig` class, which violates the
coding convention requiring all class member variables to have explicit type
annotations. Add a class-level type annotation for `engine_cfg` (with type
`Any`) in the class body before the `__init__` method to match the existing code
style in `src/**/*.py`.
In `@src/onnxocr/onnx_paddleocr.py`:
- Around line 28-49: Add type annotations to the function signatures for
_normalize_ppocrv6_size and _build_ppocrv6_defaults (and the function referenced
at line 97) to comply with the project's coding guidelines. For
_normalize_ppocrv6_size, annotate the model_name and model_size parameters with
appropriate types (likely Optional[str] or str | None) and the return type
(likely str | None). For _build_ppocrv6_defaults, annotate the kwargs parameter
(likely dict or similar) and the return type (likely dict). Use modern Python
type annotation style consistent with the rest of the codebase, using union
syntax like X | Y instead of Union[X, Y].
In `@src/onnxocr/operators.py`:
- Line 16: Add complete type annotations to the method signatures as per Python
3.11 style. For NormalizeImage.__init__ at line 16, DetResizeForTest.__init__ at
line 43, and KeepKeys.__init__ and KeepKeys.__call__ at lines 188-191, add type
hints for all parameters (scale, mean, std, order, kwargs, etc.) and return type
annotations. Ensure each parameter has its corresponding type hint and include
the return type (typically None for __init__ and __call__ methods) following the
format of parameter_name: type -> return_type.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dbd72186-780d-4bda-b20e-b3e87ee5b12f
📒 Files selected for processing (14)
src/one_dragon/base/config/basic_model_config.pysrc/one_dragon/base/matcher/ocr/onnx_ocr_matcher.pysrc/one_dragon/base/operation/one_dragon_context.pysrc/onnxocr/inference_engine.pysrc/onnxocr/logger.pysrc/onnxocr/onnx_paddleocr.pysrc/onnxocr/operators.pysrc/onnxocr/predict_base.pysrc/onnxocr/predict_cls.pysrc/onnxocr/predict_det.pysrc/onnxocr/predict_rec.pysrc/onnxocr/predict_system.pysrc/onnxocr/readme.mdsrc/onnxocr/utils.py
🚧 Files skipped from review as they are similar to previous changes (4)
- src/onnxocr/predict_base.py
- src/onnxocr/utils.py
- src/onnxocr/predict_system.py
- src/one_dragon/base/matcher/ocr/onnx_ocr_matcher.py
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/onnxocr/onnx_paddleocr.py (1)
97-134:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win函数内再次导入
log会导致UnboundLocalError。在
ocr中,Line 133 的from one_dragon.utils.log_utils import log会把log绑定为函数局部变量;因此 Line 99 的log.warning(...)会在运行时触发“引用前赋值”错误。这个路径在默认cls=True且use_angle_cls=False时可直接命中。请移除函数内log导入并复用模块级 logger。💡 建议修改
- except Exception: - from one_dragon.utils.log_utils import log - log.error('OCR推理出错', exc_info=True) + except Exception: + log.error("OCR推理出错", exc_info=True) try: from one_dragon.utils import debug_utils debug_image = img[0] if isinstance(img, list) else img - debug_utils.save_debug_image(image=debug_image, prefix='ocr_error') + debug_utils.save_debug_image(image=debug_image, prefix="ocr_error") except Exception: - log.warning('保存OCR错误调试图片失败', exc_info=True) + log.warning("保存OCR错误调试图片失败", exc_info=True) raise🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/onnxocr/onnx_paddleocr.py` around lines 97 - 134, The function ocr has a local import of log at line 133 which creates a scope binding issue. When Python sees that log is assigned locally within the function (via the from...import statement in the except block), it treats all references to log in the entire function as local variable references, including the log.warning call at line 99 which appears before the import. This causes an UnboundLocalError at runtime. Remove the local import statement from the except block at line 133 and instead rely on the module-level log import that should already exist at the top of the file. The log.error call in the except block will then correctly reference the module-level logger.
🧹 Nitpick comments (4)
src/onnxocr/onnx_paddleocr.py (1)
28-49: ⚡ Quick win新增函数签名建议补齐类型注解。
_normalize_ppocrv6_size、_build_ppocrv6_defaults与ocr的参数当前是未注解状态,和src/**/*.py的类型约定不一致。建议至少为参数与返回值补上明确类型。
As per coding guidelines:src/**/*.py要求“所有函数签名和类成员变量必须有类型注解”,并使用list[str]/X | Y风格。Also applies to: 97-97
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/onnxocr/onnx_paddleocr.py` around lines 28 - 49, Add type annotations to the function signatures for _normalize_ppocrv6_size and _build_ppocrv6_defaults (and the function referenced at line 97) to comply with the project's coding guidelines. For _normalize_ppocrv6_size, annotate the model_name and model_size parameters with appropriate types (likely Optional[str] or str | None) and the return type (likely str | None). For _build_ppocrv6_defaults, annotate the kwargs parameter (likely dict or similar) and the return type (likely dict). Use modern Python type annotation style consistent with the rest of the codebase, using union syntax like X | Y instead of Union[X, Y].Source: Coding guidelines
src/onnxocr/operators.py (1)
16-16: ⚡ Quick win本次改动触达的方法签名建议补齐类型注解。
NormalizeImage.__init__、DetResizeForTest.__init__、KeepKeys.__init__/__call__在变更后仍为无注解签名,建议按 Python 3.11 风格补齐参数与返回类型。
As per coding guidelines:src/**/*.py要求“所有函数签名和类成员变量必须有类型注解”。Also applies to: 43-43, 188-191
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/onnxocr/operators.py` at line 16, Add complete type annotations to the method signatures as per Python 3.11 style. For NormalizeImage.__init__ at line 16, DetResizeForTest.__init__ at line 43, and KeepKeys.__init__ and KeepKeys.__call__ at lines 188-191, add type hints for all parameters (scale, mean, std, order, kwargs, etc.) and return type annotations. Ensure each parameter has its corresponding type hint and include the return type (typically None for __init__ and __call__ methods) following the format of parameter_name: type -> return_type.Source: Coding guidelines
src/onnxocr/inference_engine.py (2)
157-161: ⚡ Quick win路径检查建议改为
pathlib.Path。Line 157 仍使用
os.path.exists,建议统一为Path(model_path).exists(),以贴合仓库的路径处理约定。💡 建议修改
-import os +from pathlib import Path ... - if not os.path.exists(model_path): + if not Path(model_path).exists():As per coding guidelines:
**/*.py与src/**/*.py均要求优先使用pathlib处理路径。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/onnxocr/inference_engine.py` around lines 157 - 161, Replace the `os.path.exists()` call in the model_path existence check with `Path(model_path).exists()` to align with the repository's pathlib conventions. Import `Path` from `pathlib` at the top of the file if not already imported, then update the condition that raises FileNotFoundError to use `Path(model_path).exists()` instead of `os.path.exists(model_path)` for consistency with the codebase's path handling standards.Source: Coding guidelines
97-99: ⚡ Quick win
ProviderConfig的成员变量建议补齐类型注解。
self.engine_cfg在 Line 98 被赋值,但类内未声明成员类型,和当前src/**/*.py约定不一致。建议在类体内显式声明,例如engine_cfg: Any。
As per coding guidelines:src/**/*.py要求“所有函数签名和类成员变量必须有类型注解”。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/onnxocr/inference_engine.py` around lines 97 - 99, The class member variable `engine_cfg` lacks a type annotation at the class level in the `ProviderConfig` class, which violates the coding convention requiring all class member variables to have explicit type annotations. Add a class-level type annotation for `engine_cfg` (with type `Any`) in the class body before the `__init__` method to match the existing code style in `src/**/*.py`.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/one_dragon/base/config/basic_model_config.py`:
- Around line 22-23: The ocr_use_gpu method only checks for the new
configuration key 'ocr_use_gpu' and lacks a fallback mechanism for the legacy
key 'ocr_gpu', which causes existing user configurations to silently revert to
False during upgrades. Modify the ocr_use_gpu method to first attempt retrieving
the value from the new key 'ocr_use_gpu', then fall back to the old key
'ocr_gpu' if the new key is not found, and only default to False if neither key
exists in the configuration.
In `@src/one_dragon/base/operation/one_dragon_context.py`:
- Around line 487-497: The current code assigns the OnnxOcrMatcher instance to
ocr_service.ocr_matcher and cv_service.ocr before calling init_model, which
means if init_model fails and returns False, the global references will hold an
uninitialized matcher causing subsequent OCR operations to fail. Move the
assignments of self.ocr_service.ocr_matcher and self.cv_service.ocr (currently
after self.ocr.overlay_debug_bus assignment) to execute only after init_model is
called and succeeds, so that global references only get a fully initialized and
ready OCR matcher.
In `@src/onnxocr/predict_cls.py`:
- Line 22: The log.info call uses a placeholder-based string formatting style
that triggers a static check error (PLE1205) and violates the repository's
coding guidelines. Convert the log message to use f-string formatting instead by
replacing the placeholder string with an f-string and removing the separate
argument, so that the message string with the embedded variable reference is
passed directly to the log.info method.
---
Outside diff comments:
In `@src/onnxocr/onnx_paddleocr.py`:
- Around line 97-134: The function ocr has a local import of log at line 133
which creates a scope binding issue. When Python sees that log is assigned
locally within the function (via the from...import statement in the except
block), it treats all references to log in the entire function as local variable
references, including the log.warning call at line 99 which appears before the
import. This causes an UnboundLocalError at runtime. Remove the local import
statement from the except block at line 133 and instead rely on the module-level
log import that should already exist at the top of the file. The log.error call
in the except block will then correctly reference the module-level logger.
---
Nitpick comments:
In `@src/onnxocr/inference_engine.py`:
- Around line 157-161: Replace the `os.path.exists()` call in the model_path
existence check with `Path(model_path).exists()` to align with the repository's
pathlib conventions. Import `Path` from `pathlib` at the top of the file if not
already imported, then update the condition that raises FileNotFoundError to use
`Path(model_path).exists()` instead of `os.path.exists(model_path)` for
consistency with the codebase's path handling standards.
- Around line 97-99: The class member variable `engine_cfg` lacks a type
annotation at the class level in the `ProviderConfig` class, which violates the
coding convention requiring all class member variables to have explicit type
annotations. Add a class-level type annotation for `engine_cfg` (with type
`Any`) in the class body before the `__init__` method to match the existing code
style in `src/**/*.py`.
In `@src/onnxocr/onnx_paddleocr.py`:
- Around line 28-49: Add type annotations to the function signatures for
_normalize_ppocrv6_size and _build_ppocrv6_defaults (and the function referenced
at line 97) to comply with the project's coding guidelines. For
_normalize_ppocrv6_size, annotate the model_name and model_size parameters with
appropriate types (likely Optional[str] or str | None) and the return type
(likely str | None). For _build_ppocrv6_defaults, annotate the kwargs parameter
(likely dict or similar) and the return type (likely dict). Use modern Python
type annotation style consistent with the rest of the codebase, using union
syntax like X | Y instead of Union[X, Y].
In `@src/onnxocr/operators.py`:
- Line 16: Add complete type annotations to the method signatures as per Python
3.11 style. For NormalizeImage.__init__ at line 16, DetResizeForTest.__init__ at
line 43, and KeepKeys.__init__ and KeepKeys.__call__ at lines 188-191, add type
hints for all parameters (scale, mean, std, order, kwargs, etc.) and return type
annotations. Ensure each parameter has its corresponding type hint and include
the return type (typically None for __init__ and __call__ methods) following the
format of parameter_name: type -> return_type.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dbd72186-780d-4bda-b20e-b3e87ee5b12f
📒 Files selected for processing (14)
src/one_dragon/base/config/basic_model_config.pysrc/one_dragon/base/matcher/ocr/onnx_ocr_matcher.pysrc/one_dragon/base/operation/one_dragon_context.pysrc/onnxocr/inference_engine.pysrc/onnxocr/logger.pysrc/onnxocr/onnx_paddleocr.pysrc/onnxocr/operators.pysrc/onnxocr/predict_base.pysrc/onnxocr/predict_cls.pysrc/onnxocr/predict_det.pysrc/onnxocr/predict_rec.pysrc/onnxocr/predict_system.pysrc/onnxocr/readme.mdsrc/onnxocr/utils.py
🚧 Files skipped from review as they are similar to previous changes (4)
- src/onnxocr/predict_base.py
- src/onnxocr/utils.py
- src/onnxocr/predict_system.py
- src/one_dragon/base/matcher/ocr/onnx_ocr_matcher.py
🛑 Comments failed to post (3)
src/one_dragon/base/config/basic_model_config.py (1)
22-23:
⚠️ Potential issue | 🟠 Major | ⚡ Quick win为
ocr_use_gpu增加旧配置键回退,避免升级后 GPU 设置被静默重置Line 23 仅读取
ocr_use_gpu。如果用户本地配置仍是旧键ocr_gpu,这里会回落到False,导致 OCR 从 GPU 静默降级到 CPU。💡建议修复
`@property` def ocr_use_gpu(self) -> bool: - return self.get('ocr_use_gpu', False) + return self.get('ocr_use_gpu', self.get('ocr_gpu', False))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/one_dragon/base/config/basic_model_config.py` around lines 22 - 23, The ocr_use_gpu method only checks for the new configuration key 'ocr_use_gpu' and lacks a fallback mechanism for the legacy key 'ocr_gpu', which causes existing user configurations to silently revert to False during upgrades. Modify the ocr_use_gpu method to first attempt retrieving the value from the new key 'ocr_use_gpu', then fall back to the old key 'ocr_gpu' if the new key is not found, and only default to False if neither key exists in the configuration.src/one_dragon/base/operation/one_dragon_context.py (1)
487-497:
⚠️ Potential issue | 🟠 Major | ⚡ Quick win仅在模型初始化成功后再替换全局 OCR 引用
当前在 Line 495-497 先替换
ocr_service/cv_service,再执行初始化。若init_model失败(该方法会返回False),全局会持有未就绪 matcher,后续 OCR 路径会持续异常。💡建议修复
- self.ocr = OnnxOcrMatcher( + new_ocr = OnnxOcrMatcher( OnnxOcrParam( ocr_model_name=self.model_config.ocr, use_gpu=self.model_config.ocr_use_gpu, det_limit_side_len=max(self.project_config.screen_standard_width, self.project_config.screen_standard_height), ) ) - self.ocr.overlay_debug_bus = self.overlay_debug_bus - self.ocr_service.ocr_matcher = self.ocr - if 'cv_service' in self.__dict__: - self.cv_service.ocr = self.ocr - self.ocr.init_model( + new_ocr.overlay_debug_bus = self.overlay_debug_bus + ok = new_ocr.init_model( ghproxy_url=self.env_config.gh_proxy_url if self.env_config.is_gh_proxy else None, proxy_url=self.env_config.personal_proxy if self.env_config.is_personal_proxy else None, ) + if not ok: + return + self.ocr = new_ocr + self.ocr_service.ocr_matcher = new_ocr + if 'cv_service' in self.__dict__: + self.cv_service.ocr = new_ocr🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/one_dragon/base/operation/one_dragon_context.py` around lines 487 - 497, The current code assigns the OnnxOcrMatcher instance to ocr_service.ocr_matcher and cv_service.ocr before calling init_model, which means if init_model fails and returns False, the global references will hold an uninitialized matcher causing subsequent OCR operations to fail. Move the assignments of self.ocr_service.ocr_matcher and self.cv_service.ocr (currently after self.ocr.overlay_debug_bus assignment) to execute only after init_model is called and succeeds, so that global references only get a fully initialized and ready OCR matcher.src/onnxocr/predict_cls.py (1)
22-22:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick win日志格式建议改为 f-string,避免静态检查报错
Line 22 当前写法会触发 PLE1205;改成 f-string 可同时满足仓库格式规范并消除该告警。
💡建议修复
- log.info("Classification model loaded: {}", args.cls_model_dir) + log.info(f"Classification model loaded: {args.cls_model_dir}")As per coding guidelines,
**/*.py要求 “Prefer f-string for string formatting”。🧰 Tools
🪛 Ruff (0.15.17)
[error] 22-22: Too many arguments for
loggingformat string(PLE1205)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/onnxocr/predict_cls.py` at line 22, The log.info call uses a placeholder-based string formatting style that triggers a static check error (PLE1205) and violates the repository's coding guidelines. Convert the log message to use f-string formatting instead by replacing the placeholder string with an f-string and removing the separate argument, so that the message string with the embedded variable reference is passed directly to the log.info method.Sources: Coding guidelines, Linters/SAST tools
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/one_dragon/base/matcher/ocr/onnx_ocr_matcher.py (1)
119-138: ⚡ Quick win补齐新增导出字段的类型契约。
Line 119 新增的
self.ocr_model_size还没有成员类型标注,to_dict()也缺少返回类型;这会让新增字段的导出契约不够明确。建议调整
- self.ocr_model_size = ocr_model_size + self.ocr_model_size: str = ocr_model_size @@ - def to_dict(self): + def to_dict(self) -> dict[str, bool | float | str]:As per coding guidelines,
src/**/*.py: "All function signatures and class member variables must have type annotations; uselist[str],X | Ystyle annotations".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/one_dragon/base/matcher/ocr/onnx_ocr_matcher.py` around lines 119 - 138, The new class member variable ocr_model_size on line 119 is missing a type annotation, and the to_dict() method lacks a return type annotation. Add a type annotation to the ocr_model_size member variable to specify its expected type, and add a return type annotation to the to_dict() method to indicate it returns a dictionary. Follow the coding guidelines by using standard type annotation syntax such as dict[str, Any] for the return type to match the existing codebase style.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/one_dragon/base/matcher/ocr/onnx_ocr_matcher.py`:
- Around line 40-68: The current implementation of get_ocr_model_dict_name lacks
a reliable model-to-dictionary mapping, causing runtime failures when returning
None or empty strings. Create a constant dictionary mapping model names (e.g.,
ppocrv5) to their corresponding dictionary filenames (e.g., ppocrv5_dict.txt),
then modify get_ocr_model_dict_name to first check this mapping before falling
back to local filesystem scanning for backward compatibility. This ensures
get_final_file_list always includes the correct dictionary file in its returned
file list, and prevents os.path.join from constructing invalid directory paths
due to empty string filenames. Additionally, add explicit return type
annotations to the to_dict method and add explicit type declarations for the
self.ocr_model_size attribute to comply with Python 3.11+ type annotation
standards.
---
Nitpick comments:
In `@src/one_dragon/base/matcher/ocr/onnx_ocr_matcher.py`:
- Around line 119-138: The new class member variable ocr_model_size on line 119
is missing a type annotation, and the to_dict() method lacks a return type
annotation. Add a type annotation to the ocr_model_size member variable to
specify its expected type, and add a return type annotation to the to_dict()
method to indicate it returns a dictionary. Follow the coding guidelines by
using standard type annotation syntax such as dict[str, Any] for the return type
to match the existing codebase 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 00e841fe-27b8-4d39-a436-07284915cc56
📒 Files selected for processing (3)
src/one_dragon/base/config/basic_model_config.pysrc/one_dragon/base/matcher/ocr/onnx_ocr_matcher.pysrc/onnxocr/predict_det.py
🚧 Files skipped from review as they are similar to previous changes (2)
- src/one_dragon/base/config/basic_model_config.py
- src/onnxocr/predict_det.py
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/one_dragon/base/matcher/ocr/onnx_ocr_matcher.py (1)
123-123: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win补齐新增字段相关的类型注解以满足仓库规范
self.ocr_model_size和to_dict当前缺少显式类型注解,和src/**/*.py的强约束不一致。建议至少补上成员变量注解与to_dict返回类型。建议修改
- self.ocr_model_size = ocr_model_size + self.ocr_model_size: str = ocr_model_size - def to_dict(self): + def to_dict(self) -> dict[str, str | bool | float]:As per coding guidelines,
src/**/*.py要求“所有函数签名和类成员变量必须有类型注解”,且**/*.py目标为 Python 3.11+。Also applies to: 130-141
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/one_dragon/base/matcher/ocr/onnx_ocr_matcher.py` at line 123, The new member variable self.ocr_model_size and the to_dict method are missing type annotations, which violates the repository's Python type annotation requirements. Add an explicit type annotation to the self.ocr_model_size assignment based on the type of the ocr_model_size parameter, and add a return type annotation to the to_dict method (typically Dict or dict depending on Python version) to comply with the src/**/*.py coding guidelines that require all class member variables and function signatures to have type annotations.Source: Coding guidelines
src/onnxocr/onnx_paddleocr.py (2)
97-97: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win补齐
ocr()的参数类型和结果契约。这是公开推理入口,但
img/det/rec/cls仍未标注类型,-> list也没有表达下游依赖的嵌套结果结构。建议补充参数类型、精确返回类型,并加一段中文 Google 风格 docstring 说明 det/rec/cls 各组合的返回形状。As per coding guidelines,src/**/*.py中所有函数签名必须有类型注解,职责较重函数应有注释。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/onnxocr/onnx_paddleocr.py` at line 97, The public inference entry point method ocr() in the OnnxPaddleOCR class lacks complete type annotations and documentation. Add type annotations to all parameters (img should be annotated for its expected image input type, and det/rec/cls should be annotated as bool), replace the generic `-> list` return annotation with a precise type that reflects the nested structure of results returned by the function, and add a comprehensive Google-style docstring in Chinese that documents the function's purpose, parameters, return structure, and explains how the return shape varies based on different combinations of det/rec/cls parameter values.Source: Coding guidelines
78-91: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift把
ONNXPaddleOcr.__init__改成显式参数签名。当前继续用
**kwargs承接ocr_model_size、模型路径、开关和阈值等配置,新增 v6 默认值与上游OnnxOcrParam.to_dict()的契约无法被类型检查覆盖。建议将支持的参数逐个声明,并在函数内组装Namespace。As per coding guidelines,src/**/*.py中构造函数必须显式声明参数,且不要使用**kwargs。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/onnxocr/onnx_paddleocr.py` around lines 78 - 91, The `ONNXPaddleOcr.__init__` method currently accepts `**kwargs` which prevents type checking and makes the interface unclear. Replace the `**kwargs` parameter with explicit parameter declarations for all supported configuration options (such as ocr_model_size, model paths, switches, and thresholds), and update the method body to use these explicit parameters instead of the kwargs dictionary. Specifically, remove `**kwargs` from the function signature, declare each supported parameter explicitly with appropriate defaults, and then update the line where `params.__dict__.update(**kwargs)` is called to instead use the explicit parameter values.Source: Coding guidelines
♻️ Duplicate comments (1)
src/one_dragon/base/matcher/ocr/onnx_ocr_matcher.py (1)
93-95:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
dict_name的失败时机前置会阻断首次安装流程Line 93-95 在参数构造阶段即依赖本地字典探测并抛错;当模型尚未下载到本地时,这会在
OnnxOcrParam初始化就失败,导致后续下载流程无法启动。建议把“模型名→字典名”的确定逻辑做稳定兜底(本地扫描仅作为兼容分支),避免把首次安装场景判定为异常。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/one_dragon/base/matcher/ocr/onnx_ocr_matcher.py` around lines 93 - 95, The issue is that in the OnnxOcrMatcher class, the call to get_ocr_model_dict_name in the parameter construction phase immediately raises a FileNotFoundError when dict_name is None, which blocks the download flow when models haven't been downloaded locally yet. Instead of failing during OnnxOcrParam initialization, refactor the "model name to dict name" resolution logic to provide a robust fallback mechanism where local file scanning is treated as a compatibility branch rather than the primary requirement. Consider deferring the dict_name validation to a later stage or providing a sensible default that allows the initial download process to proceed, ensuring first-time installation scenarios are not treated as abnormal errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/one_dragon/base/matcher/ocr/onnx_ocr_matcher.py`:
- Line 123: The new member variable self.ocr_model_size and the to_dict method
are missing type annotations, which violates the repository's Python type
annotation requirements. Add an explicit type annotation to the
self.ocr_model_size assignment based on the type of the ocr_model_size
parameter, and add a return type annotation to the to_dict method (typically
Dict or dict depending on Python version) to comply with the src/**/*.py coding
guidelines that require all class member variables and function signatures to
have type annotations.
In `@src/onnxocr/onnx_paddleocr.py`:
- Line 97: The public inference entry point method ocr() in the OnnxPaddleOCR
class lacks complete type annotations and documentation. Add type annotations to
all parameters (img should be annotated for its expected image input type, and
det/rec/cls should be annotated as bool), replace the generic `-> list` return
annotation with a precise type that reflects the nested structure of results
returned by the function, and add a comprehensive Google-style docstring in
Chinese that documents the function's purpose, parameters, return structure, and
explains how the return shape varies based on different combinations of
det/rec/cls parameter values.
- Around line 78-91: The `ONNXPaddleOcr.__init__` method currently accepts
`**kwargs` which prevents type checking and makes the interface unclear. Replace
the `**kwargs` parameter with explicit parameter declarations for all supported
configuration options (such as ocr_model_size, model paths, switches, and
thresholds), and update the method body to use these explicit parameters instead
of the kwargs dictionary. Specifically, remove `**kwargs` from the function
signature, declare each supported parameter explicitly with appropriate
defaults, and then update the line where `params.__dict__.update(**kwargs)` is
called to instead use the explicit parameter values.
---
Duplicate comments:
In `@src/one_dragon/base/matcher/ocr/onnx_ocr_matcher.py`:
- Around line 93-95: The issue is that in the OnnxOcrMatcher class, the call to
get_ocr_model_dict_name in the parameter construction phase immediately raises a
FileNotFoundError when dict_name is None, which blocks the download flow when
models haven't been downloaded locally yet. Instead of failing during
OnnxOcrParam initialization, refactor the "model name to dict name" resolution
logic to provide a robust fallback mechanism where local file scanning is
treated as a compatibility branch rather than the primary requirement. Consider
deferring the dict_name validation to a later stage or providing a sensible
default that allows the initial download process to proceed, ensuring first-time
installation scenarios are not treated as abnormal errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5acbc21c-a1dc-43e9-aa21-79e1e05a1f61
📒 Files selected for processing (5)
src/one_dragon/base/matcher/ocr/onnx_ocr_matcher.pysrc/one_dragon/base/operation/one_dragon_context.pysrc/onnxocr/onnx_paddleocr.pysrc/onnxocr/predict_base.pysrc/onnxocr/predict_system.py
💤 Files with no reviewable changes (1)
- src/onnxocr/predict_base.py
🚧 Files skipped from review as they are similar to previous changes (2)
- src/onnxocr/predict_system.py
- src/one_dragon/base/operation/one_dragon_context.py
…otations/docstring for ocr()
30958ea to
efcffba
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/one_dragon/base/matcher/ocr/onnx_ocr_matcher.py (1)
116-125: 💤 Low value类型标注过于宽泛
经过 if-else 链后,
ocr_model_size必定是str类型而非None。建议将类型标注修正为str以提高类型准确性。♻️ 建议修复
- self.ocr_model_size: str | None = ocr_model_size + self.ocr_model_size: str = ocr_model_size🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/one_dragon/base/matcher/ocr/onnx_ocr_matcher.py` around lines 116 - 125, The type annotation for self.ocr_model_size is declared as str | None, but after the if-else chain evaluating ocr_model_size, it will always be assigned a string value (either 'medium', 'tiny', 'small', or defaults to 'small'). Change the type annotation on the self.ocr_model_size assignment from str | None to just str to accurately reflect that the variable will never be None at that point.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/one_dragon/base/matcher/ocr/onnx_ocr_matcher.py`:
- Around line 116-125: The type annotation for self.ocr_model_size is declared
as str | None, but after the if-else chain evaluating ocr_model_size, it will
always be assigned a string value (either 'medium', 'tiny', 'small', or defaults
to 'small'). Change the type annotation on the self.ocr_model_size assignment
from str | None to just str to accurately reflect that the variable will never
be None at that point.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5b7ea60e-882d-4594-895c-802d8d23524f
📒 Files selected for processing (5)
src/one_dragon/base/matcher/ocr/onnx_ocr_matcher.pysrc/one_dragon/base/operation/one_dragon_context.pysrc/onnxocr/onnx_paddleocr.pysrc/onnxocr/predict_base.pysrc/onnxocr/predict_system.py
💤 Files with no reviewable changes (1)
- src/onnxocr/predict_base.py
🚧 Files skipped from review as they are similar to previous changes (2)
- src/one_dragon/base/operation/one_dragon_context.py
- src/onnxocr/predict_system.py
|
模型名对齐一下 |
c5b4b5a to
243a20a
Compare
| def normalize_ocr_model_name(ocr_model_name: str) -> str: | ||
| """ | ||
| 规范化 OCR 模型名。 | ||
| """ | ||
| if ocr_model_name.startswith(PPOCRV6_MODEL_NAME): | ||
| return PPOCRV6_MODEL_NAME | ||
| return ocr_model_name | ||
|
|
||
|
|
Summary by CodeRabbit
发布说明
det_db_max_candidates);改进异常与参数解析表现。