fix(ocr): use PSM integer values directly instead of constructor #2578
Conversation
- Use integer psm value directly instead of calling tesserocr.PSM() - Fixed in both main_psm and script_readers initialization - tesserocr.PSM is a class with integer constants, not an enum Fixes docling-project#2576
|
✅ DCO Check Passed Thanks @Mulgyeol, all your commits are properly signed off. 🎉 |
|
Related Documentation Checked 3 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
I, mulgyeol <mulgyeoljung@gmail.com>, hereby add my Signed-off-by to this commit: da63a17 Signed-off-by: mulgyeol <mulgyeoljung@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes type handling for the Tesseract Page Segmentation Mode (PSM) parameter by removing unnecessary tesserocr.PSM() enum wrapper calls.
- Removes
tesserocr.PSM()wrapper aroundself.options.psmwhen initializing Tesseract readers - Adds test coverage for using PSM parameter with
TesseractOcrOptions
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| docling/models/tesseract_ocr_model.py | Removes tesserocr.PSM() wrapper calls, allowing integer PSM values to be passed directly to the API |
| tests/test_e2e_ocr_conversion.py | Adds test case for TesseractOcrOptions(psm=3) to verify PSM parameter handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| tesserocr.PSM(self.options.psm) | ||
| if self.options.psm is not None | ||
| else tesserocr.PSM.AUTO | ||
| self.options.psm if self.options.psm is not None else tesserocr.PSM.AUTO |
There was a problem hiding this comment.
Type inconsistency: self.options.psm is an Optional[int] but is being mixed with tesserocr.PSM.AUTO enum value. The fallback should use an integer value (e.g., 3 for AUTO) or self.options.psm should be converted to the enum type. This creates a type mismatch where the ternary expression returns different types depending on the condition.
| self.options.psm if self.options.psm is not None else tesserocr.PSM.AUTO | |
| tesserocr.PSM(self.options.psm) if self.options.psm is not None else tesserocr.PSM.AUTO |
| psm=self.options.psm | ||
| if self.options.psm is not None | ||
| else tesserocr.PSM.AUTO, |
There was a problem hiding this comment.
Type inconsistency: Same issue as line 100 - self.options.psm (an Optional[int]) is being mixed with tesserocr.PSM.AUTO enum value. The ternary expression returns different types, which could cause issues depending on how the tesserocr API handles type checking.
| psm=self.options.psm | |
| if self.options.psm is not None | |
| else tesserocr.PSM.AUTO, | |
| psm=tesserocr.PSM(self.options.psm) if self.options.psm is not None else tesserocr.PSM.AUTO, |
2 participants
Fixes TypeError when TesseractOcrOptions is initialized with an explicit PSM parameter.
Changes
tesserocr.PSM()constructormain_psminitialization (line 100) andscript_readersinitialization (line 198)tesserocr.PSMis a class with integer constants, not a callable enumTesseractOcrOptions(psm=3)to prevent future issuesBackground
The bug was introduced in v2.56.0 when PSM configurability was added. The code incorrectly attempted to construct a PSM enum from an integer, but
tesserocr.PSMconstants are already integers and should be used directly.Issue resolved by this Pull Request:
Resolves #2576
Checklist: