Use docling package with default PdfConverter
#192
Conversation
Keep `pdfminer` in PdfMinerConverter to allow for comparisons between 2 alternatives
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Fix Detected |
|---|---|---|
 |
Silent module import failure ▹ view | ✅ |
|
Missing Error Handling for Failed Conversion ▹ view | ✅ |
 |
Poor separation of concerns in document reading logic ▹ view | ✅ |
|
Redundant Variable Initialization Masks Errors ▹ view | ✅ |
Files scanned
| File Path | Reviewed |
|---|---|
| examples/convert_document.py | ✅ |
| tapeagents/tools/document_reader.py | ✅ |
| tapeagents/tools/converters.py | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Need a new review? Comment
/korbit-reviewon this PR and I'll review your latest changes.Korbit Guide: Usage and Customization
Interacting with Korbit
- You can manually ask Korbit to review your PR using the
/korbit-reviewcommand in a comment at the root of your PR.- You can ask Korbit to generate a new PR description using the
/korbit-generate-pr-descriptioncommand in any comment on your PR.- Too many Korbit comments? I can resolve all my comment threads if you use the
/korbit-resolvecommand in any comment on your PR.- Chat with Korbit on issues we post by tagging @korbit-ai in your reply.
- Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.
Customizing Korbit
- Check out our docs on how you can make Korbit work best for you and your team.
- Customize Korbit for your organization through the Korbit Console.
Feedback and Support
|
/korbit-review |
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Fix Detected |
|---|---|---|
 |
Replace print with proper logging ▹ view | |
|
Inconsistent Logger Usage ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| examples/convert_document.py | ✅ |
| tapeagents/tools/document_reader.py | ✅ |
| tapeagents/tools/converters.py | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Need a new review? Comment
/korbit-reviewon this PR and I'll review your latest changes.Korbit Guide: Usage and Customization
Interacting with Korbit
- You can manually ask Korbit to review your PR using the
/korbit-reviewcommand in a comment at the root of your PR.- You can ask Korbit to generate a new PR description using the
/korbit-generate-pr-descriptioncommand in any comment on your PR.- Too many Korbit comments? I can resolve all my comment threads if you use the
/korbit-resolvecommand in any comment on your PR.- Chat with Korbit on issues we post by tagging @korbit-ai in your reply.
- Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.
Customizing Korbit
- Check out our docs on how you can make Korbit work best for you and your team.
- Customize Korbit for your organization through the Korbit Console.
Current Korbit Configuration
General Settings
Setting Value Review Schedule Automatic excluding drafts Max Issue Count 10 Automatic PR Descriptions ✅ Issue Categories
Category Enabled Documentation ✅ Logging ✅ Error Handling ✅ Readability ✅ Design ✅ Performance ✅ Security ✅ Functionality ✅ Feedback and Support
Note
Korbit Pro is free for open source projects 🎉
Looking to add Korbit to your team? Get started with a free 2 week trial here
danieltremblay
force-pushed
the
dtremblay/add_docling
branch
from
12e0b08 to
6abb531
Compare
|
High-level question: Docling landing page states that it's capable of parsing XLSX, DOCX, and others, not just PDF. Could we replace our entire convert_document.py borrowed from MS Autogen with it? |
|
I would propose to rename the DocumentReader to LegacyDocumentReader and create a new DocumentReader(Tool) completely around the Docling. @danieltremblay, does it make sense to do it in this PR or make it a separate one? |
@ollmer Sure, that can be done. We could replace the underlying FileConverter class for one that uses Docling for more document types. I'm tempted to do it in a separate PR so others can experiment with the Docling PDF converter in |
Keep
pdfminerin PdfMinerConverter to allow for comparisons between 2 alternativesdoclinglibrary is pinned at 2.15.0 (January 2025) because later versions have a strictertransformers=~4.42.0dependency which conflicts with our optionalfinetunegroup that requirestransformers~=4.45If this is an issue, we could look into Docling MCP or look into applying conflict-groups (astral-sh/uv#8976)
Description by Korbit AI
What change is being made?
Introduce the
doclingpackage as the default PDF converter in the configuration, replacing the use ofpdfminerwhere applicable, and integrate the respective changes across the codebase to support this transition.Why are these changes being made?
The
doclingpackage provides enhanced functionality for PDF conversion, including better handling of tables, making it a more suitable default option overpdfminer. This change aims to improve document conversion accuracy and efficiency, ensuring users get precise and detailed content extraction results. Additionally, maintaining compatibility with bothdoclingandpdfminerprovides flexibility in handling PDF documents.