Skip to content
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

Should the second "if" keyword in adapt_bbox from layout_utils.py rather be an "elif" keyword ? #362

Closed
Raphilanthrope opened this issue Nov 18, 2024 · 2 comments
Assignees
Labels
question Further information is requested

Comments

@Raphilanthrope
Copy link

Question

I think I identified a typo based on a comment and the logic of the code itself.

In "adapt_bbox" from "layout_utils.py", there are three lines at the first function indentation level:
if not (cluster["type"] in [DocItemLabel.TABLE, DocItemLabel.PICTURE]):;
if cluster["type"] == DocItemLabel.PICTURE: and
else: ## A table

Based on the comment ## A table and the fact that there is to my understanding other classes that TABLE and PICTURES leading to a re-definition of new_bbox in the else statement when handling a cluster's class different from a table and a picture, I think the second if is meant to be an elif keyword., i.e. we would write:
elif cluster["type"] == DocItemLabel.PICTURE:

Am I right ?

Thank you for this great tool by the way :)

@Raphilanthrope Raphilanthrope added the question Further information is requested label Nov 18, 2024
@PeterStaar-IBM
Copy link
Contributor

@Raphilanthrope Make a quick PR with the fix, this could indeed be a bug.

Raphilanthrope added a commit to Raphilanthrope/docling that referenced this issue Nov 22, 2024
See issue raised in DS4SD#362 (comment)

Signed-off-by: Raphaël M.J.I. Larsen <39514032+Raphilanthrope@users.noreply.github.com>
@cau-git
Copy link
Contributor

cau-git commented Dec 18, 2024

@Raphilanthrope This is obsolete since docling 2.13.0 because the layout_utils code is entirely replaced.

@cau-git cau-git closed this as completed Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants