Trim CSV blank runs without repeatedly shifting the row list - #2450
Merged
Conversation
CSV 清理函数逐次 pop(0) 或 pop(1)，每次都移动剩余元素，导致大量前导空行或表头后空行的转换耗时呈平方级增长。 改为先定位表头、数据起点和末尾，再逆序删除空行区间；保留内部空行、表头及行对象，不改变列宽或转义逻辑。新增四个十万空行场景，覆盖前导、表头后、尾部与全空输入，同时检查内部空行和宽行保留行为。 验证：37 项 CSV 测试通过；511 种空行排列与基线函数输出和对象身份一致；pre-commit 全文件检查及 diff 空白检查通过。Windows / Python 3.13.15 的合成输入测量中，十万前导空行的真实转换入口三次中位数从 0.6953 秒降至 0.0084 秒。 未运行整个项目的可选格式、远程服务或付费模型测试。性能测量针对合成空行输入，不代表普通文档的整体提速。
Contributor
Author
|
@microsoft-github-policy-service agree |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The implementation preserves existing semantics, resolves the quadratic behavior, and includes focused regression coverage.
Pull request overview
Optimizes CSV blank-row trimming from quadratic to linear behavior while preserving conversion output and row identity.
Changes:
- Deletes blank-row runs using slices rather than repeated list shifts.
- Adds regression coverage for long leading, post-header, trailing, and all-blank inputs.
File summaries
| File | Description |
|---|---|
packages/markitdown/src/markitdown/converters/_csv_converter.py |
Implements efficient in-place blank-run trimming. |
packages/markitdown/tests/test_csv_blank_runs.py |
Tests long blank runs and content preservation. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2449.
CSV conversion repeatedly shifts the row list when removing blank lines before the header or immediately after it. A long blank run therefore takes quadratic time even though the resulting table is tiny.
This change finds the boundaries first and deletes each run in one slice. The helper still mutates the original list and preserves the header, internal blank rows, and existing row objects.
Four conversion tests cover 100,000 blank lines before the header, after the header, at the end, and throughout the file. The table cases also check that an internal blank row and a wider data row survive. There is no timing assertion in the tests.
Validation on Windows / Python 3.13.15:
pre-commit run --all-filespassed.The full optional-format and remote-service test suite was not run locally.
Prepared with OpenAI Codex assistance.