Refactoring PDF loaders: 02 PyMuPDF

[pprados]

• Refactoring PDF loaders step 2: "community: Refactoring PDF loaders to standardize approaches"

• Description: Update PyMuPDFParser/Loader

• Twitter handle: pprados

This is one part of a larger Pull Request (PR) that is too large to be submitted all at once.
This specific part focuses to prepare the update of all parsers.

For more details, see PR 28970.

@eyurtsev it's the continuation of PDFLoader modifications.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
langchain	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 14, 2025 10:29am

[pprados]

@eyurtsev I rebase the code with master ;-)

[eyurtsev]

Great will take a look in the AM

[eyurtsev]

Left two major comment, a few stylistic comments and some nits.

Let's tackle the two major comments:

1. Define the standardized structure of metadata
2. Create a dedicated ImageParser which is a blob parser

[eyurtsev]

Users will not understand what this means from looking at the API reference docs. Could we improve until this is self-explanation from just looking at the API Reference?

Perhaps a type update to: Optional[Literal['markdown-link', 'html-img']] and make the literal names more specific?

For example,

• None = return the content as is
• markdown-link = wrap the content into a markdown link, w/ link pointing to...
• html-img = wrap the content as the alt text of an tag and link to ...

Also consider a clash against a potential future implementation: The new implementation accepts a screenshot of an entire PDF page and create a markdown representation of it. (i.e., the output is markdown here, but the has a very different meaning now and we don't want to rewrap it all as a markdown link).

Perhaps renaming format to wrap_as would be helpful for communicating what this does?

[pprados]

I'm not sure wrap_as is clearer than format.

[eyurtsev]

• Could we remove the intermediate abstraction? It doesn't serve any user facing purpose in this case -- our unit tests can check that parsers conform to an interface?
• Why are extract_images and images_parser definable independently? Why not use just images_parser: Optional[BaseImageBlobParser] = None for any new implementations?

[pprados]

• This is because, when 10 other PRs are published with other parsers, this class will become meaningful and will be shared with all image management-compatible parsers.
• It's for compatibility reasons. I prefer to have only images_parser: Optional[BaseImageBlobParser] = None

It is possible to declare extract_images deprecated, with, implicitly, the use of RapidOCRBlobParser() if, and only if, extract_images=True, with an alert.

What do you think?

[pprados]

@eyurtsev
Currently, core sources generate lint errors. There's nothing I can do about it.

[ "langchain_core" = "" ] || poetry run ruff format langchain_core --diff
1 file would be reformatted, 162 files already formatted
--- langchain_core/documents/base.py
+++ langchain_core/documents/base.py