Skip to content

Conversation

@schwarbf
Copy link
Contributor

@schwarbf schwarbf commented Dec 10, 2025

This PR aims to close the open issue 453.

I tried to do as little code changes as possible. I did the following:

  • I started taking the ruff config from the docling repository
  • Using ruff now as a replacement for black, isort, autoflake and flake8
  • Ran ruff format (formatted approx. 30 files) and ruff check --fix (fixed approx. 50 warnings)
  • Excluded the test/data from ruff
  • Added some rules to the ignore list that would have affected > 3 changes. Feel free to remove any again to see possible changes.
  • Set the line length again to 120 in pyproject.toml (was 120 in .flake8 file, but 88 in pyproject.toml)

FYI: When checking which commit added ruff to docling, you can see that 5458a88
also changed a significant number of files and added some rules to the ignore list.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 10, 2025

DCO Check Passed

Thanks @schwarbf, all your commits are properly signed off. 🎉

@dosubot
Copy link

dosubot bot commented Dec 10, 2025

Related Documentation

Checked 7 published document(s) in 0 knowledge base(s). No updates required.

How did I do? Any feedback?  Join Discord

@mergify
Copy link

mergify bot commented Dec 10, 2025

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🟢 Enforce conventional commit

Wonderful, this rule succeeded.

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

  • title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.+\))?(!)?:

🟢 Require two reviewer for test updates

Wonderful, this rule succeeded.

When test data is updated, we require two reviewers

  • #approved-reviews-by >= 2

@schwarbf schwarbf changed the title Replace black and isort by ruff Replace black and isort by ruff Dec 10, 2025
@schwarbf schwarbf changed the title Replace black and isort by ruff chore:Replace black and isort by ruff Dec 10, 2025
@schwarbf schwarbf changed the title chore:Replace black and isort by ruff chore: Replace black and isort by ruff Dec 10, 2025
Comment on lines 31 to 36
part_name: str | None = Field(default=None)
docstring: str | None = Field(default=None)
sha256: int | None = Field(default=None)
start_line: int | None = Field(default=None)
end_line: int | None = Field(default=None)
end_line_signature: int | None = Field(default=None)
Copy link
Member

Choose a reason for hiding this comment

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

Was this suggested by Ruff?
We still support 3.9 (it may change at some point) and the | syntax is only valid from 3.10. We should therefore keep typing.Optional

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay I see. It's weird since I had actually set :

[tool.ruff]
target-version = "py39"

Anyways, I will revert this part.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! However, I've seen that the syntax is still in other places, like in test/test_page_chunker.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you please check again? I found 2 remaining spots, where the syntax dtype | None was used instead of Optional[dtype]. The respective rule UP045 is also in the ignore list such that no errors are raised in the future.

@schwarbf schwarbf force-pushed the dev/introduce-ruff-to-docling-core branch from 4c618ea to 85a1889 Compare December 12, 2025 19:18
@schwarbf schwarbf changed the title chore: Replace black and isort by ruff chore: Replace black, isort, flak8 and autoflake by ruff Dec 12, 2025
@schwarbf schwarbf requested a review from ceberam December 12, 2025 20:48
@schwarbf
Copy link
Contributor Author

@ceberam PR should be ready. Let me know if you want me to rebase or merge with docling-core:main or if you want to do it yourself?

@ceberam
Copy link
Member

ceberam commented Jan 12, 2026

@schwarbf sorry for the late reply. Would you mind rebasing to the latest main?

Florian Schwarb added 17 commits January 13, 2026 11:10
Signed-off-by: Florian Schwarb <[email protected]>
Signed-off-by: Florian Schwarb <[email protected]>
Signed-off-by: Florian Schwarb <[email protected]>
Signed-off-by: Florian Schwarb <[email protected]>
Signed-off-by: Florian Schwarb <[email protected]>
…ff (i.e. no separate [tool.flake8] section

Signed-off-by: Florian Schwarb <[email protected]>
Florian Schwarb added 4 commits January 13, 2026 11:21
@ceberam ceberam force-pushed the dev/introduce-ruff-to-docling-core branch from 7b6da2d to b60b5c3 Compare January 13, 2026 10:27
@ceberam
Copy link
Member

ceberam commented Jan 13, 2026

@schwarbf I have rebased and force-pushed your commits. I'll add a couple of them to fix some small details and apply a couple of additional style rules.

@ceberam ceberam changed the title chore: Replace black, isort, flak8 and autoflake by ruff style: replace black, isort, flake8 and autoflake with ruff Jan 13, 2026
@ceberam ceberam force-pushed the dev/introduce-ruff-to-docling-core branch from d4b6f87 to a73490d Compare January 13, 2026 13:56
@schwarbf
Copy link
Contributor Author

schwarbf commented Jan 13, 2026

@schwarbf I have rebased and force-pushed your commits. I'll add a couple of them to fix some small details and apply a couple of additional style rules.

@ceberam That's fine thanks. I was going to do it tonight. Seems like we're ready to merge apart from the codecov check which was probably failing before as well?

@ceberam ceberam merged commit c73904e into docling-project:main Jan 13, 2026
11 of 12 checks passed
@ceberam
Copy link
Member

ceberam commented Jan 13, 2026

@schwarbf I have rebased and force-pushed your commits. I'll add a couple of them to fix some small details and apply a couple of additional style rules.

@ceberam That's fine thanks. I was going to do it tonight. Seems like we're ready to merge apart from the codecov check which was probably failing before as well?

All good @schwarbf , thanks for your contribution!

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.

4 participants