Skip to content

Feat/remove reference of PageLayout.elements #3943

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Mar 12, 2025

Conversation

badGarnet
Copy link
Collaborator

@badGarnet badGarnet commented Mar 5, 2025

This PR removes usage of PageLayout.elements from partition function, except for when analysis=True. This PR updates the partition logic so that PageLayout.elements_array is used everywhere to save memory and cpu cost.
Since the analysis function is intended for investigation and not for general document processing purposes, this part of the code is left for a future refactor.

PageLayout.elements uses a list to store layout elements' data while elements_array uses numpy array to store the data, which has much lower memory requirements. Using memory_profiler to test the differences is usually around 10x.

@badGarnet badGarnet changed the title Feat/remove reference of pagelayout elements Feat/remove reference of PageLayout.elements Mar 6, 2025
@badGarnet badGarnet marked this pull request as ready for review March 6, 2025 16:35
Copy link
Contributor

@MaksOpp MaksOpp left a comment

Choose a reason for hiding this comment

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

LGTM! Attaching the reports for visibility
reports.zip

Comment on lines -762 to -772
for page in final_document_layout.pages:
for el in page.elements:
el.text = el.text or ""

Copy link
Contributor

Choose a reason for hiding this comment

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

Those lines are weird so I am happy they are removed. But do you have some insights on why they were added in the first place? Removing them should not break anything right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is when and element's text can be None. But vectorized data structure cleaned that upstream so at this point there is no need for those lines


### Enhancements

- **`extract_image_block_types` now also works for CamelCase elemenet type names**. Previously `NarrativeText` and similar CamelCase element types can't be extracted using the mentioned parameter in `partition`. Now figures for those elements can be extracted like `Image` and `Table` elements
- **stop using `PageLayout.elements` to save memory and cpu cost**. Now only use `PageLayout.elements_array` throughout the partition, except when `analysis=True` where the drawing logic still uses `elements`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you recall any downstream tasks that can still be using PageLayout.elements and would need some refactor?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nothing in the unstructured lib. If user still want elements they can still get it (it is a cached property so will be computed if not already when getting that attribute)

@badGarnet badGarnet enabled auto-merge March 12, 2025 14:42
@badGarnet badGarnet added this pull request to the merge queue Mar 12, 2025
Merged via the queue into main with commit 2dceac3 Mar 12, 2025
43 checks passed
@badGarnet badGarnet deleted the feat/remove-reference-of-pagelayout-elements branch March 12, 2025 15:55
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