kaxil commented on code in PR #73926:
URL: https://github.com/apache/airflow/pull/73926#discussion_r4190122404
##########
providers/common/ai/docs/operators/document_loader.rst:
##########
@@ -73,14 +73,30 @@ Install the ``docx`` extra to parse Word documents via
pip install "apache-airflow-providers-common-ai[docx]"
-All non-empty paragraphs are concatenated into a single document per file.
+Paragraphs and tables in the document body are concatenated, in document
+order, into a single document per file. Empty paragraphs and table rows are
+skipped. Each table row becomes one line:
+
+.. code-block:: text
+
+ Quarterly results
+
+ | Region | Revenue |
+ | EMEA | 1.2M |
+
+A cell merged across columns appears once. A cell merged down several rows is
+repeated on each of those rows, so every row still reads on its own. A table
+nested inside a cell is flattened into that cell, with ``/`` between its cells
+and ``;`` between its rows. A table that ``python-docx`` cannot" read, such as
one
Review Comment:
There's a stray `"` after "cannot", so the page renders as `cannot" read`.
##########
providers/common/ai/pyproject.toml:
##########
@@ -147,7 +147,9 @@ dependencies = [
"llama-index-llms-openai>=0.6.8",
]
"pdf" = ["pypdf>=4.0.0"]
-"docx" = ["python-docx>=1.0.0"]
+# python-docx 1.1.2 is the first release whose _Row.cells walks a single row
and exposes
Review Comment:
I had this part wrong too: 1.1.1 already has the per-row `_Row.cells` and
`grid_cols_before`/`grid_cols_after` (its `_Row` class is identical to
1.1.2's). The only reason to skip 1.1.1 is its `lxml<=4.9.2` pin. Maybe:
"python-docx 1.1.1 added the per-row _Row.cells and
grid_cols_before/grid_cols_after but pins lxml<=4.9.2, which does not build on
3.12, so 1.1.2 is the first release that works."
##########
providers/common/ai/src/airflow/providers/common/ai/operators/document_loader.py:
##########
@@ -450,21 +453,68 @@ def _parse_pdf_stream(self, stream: BinaryIO) ->
list[dict[str, Any]]:
documents.append({"text": text, "metadata": {"page_number":
page_num + 1}})
return documents
- def _parse_docx_stream(self, stream: BinaryIO) -> list[dict[str, Any]]:
+ def _parse_docx_stream(self, stream: BinaryIO, *, source_hint: str) ->
list[dict[str, Any]]:
"""
- Parse a DOCX stream into documents.
+ Parse a DOCX stream into a single document.
- Extracts paragraph text only. Tables, headers, footers, and footnotes
- are not included. For richer DOCX parsing, plug in a dedicated
- extraction tool (``Unstructured``, ``docling``) as a custom parser
- backend.
+ Paragraphs and tables in the document body are extracted in document
+ order. Each table row becomes one "| cell | cell |" line, and a nested
+ table is flattened into its cell. Headers, footers, footnotes, content
+ controls, text boxes, and pending tracked insertions are not included.
"""
try:
from docx import Document
+ from docx.table import Table
except ImportError as e:
raise AirflowOptionalProviderFeatureException(e)
doc = Document(stream)
- paragraphs = [p.text for p in doc.paragraphs if p.text.strip()]
- text = "\n\n".join(paragraphs)
+ blocks = []
+ for block in doc.iter_inner_content():
+ if isinstance(block, Table):
+ try:
+ rows = self._get_docx_table_rows(block, Table)
+ except (ValueError, RecursionError) as e:
+ # python-docx walks vertical merges up recursively: a
malformed merge raises
+ # ValueError and a merge spanning about 1000 rows raises
RecursionError.
+ self.log.warning(
+ "Skipping a table in %s that python-docx could not
read: %r", source_hint, e
+ )
+ continue
+ text = "\n".join(f"| {' | '.join(cells)} |" for cells in rows)
+ else:
+ text = block.text
+ if text.strip():
+ blocks.append(text)
+ text = "\n\n".join(blocks)
return [{"text": text, "metadata": {}}]
+
+ def _get_docx_table_rows(self, table: Table, table_cls: type[Table]) ->
list[list[str]]:
+ rows = []
+ for row in table.rows:
+ cells: list[str] = [""] * row.grid_cols_before
Review Comment:
`grid_cols_before`/`grid_cols_after` (here and on line 504) come straight
from the XML, so a 36 KB .docx with `w:gridBefore w:val="10000000"` returns
about 30M characters. Capping both at `len(table.columns)` bounds it.
##########
providers/common/ai/src/airflow/providers/common/ai/operators/document_loader.py:
##########
@@ -450,21 +453,68 @@ def _parse_pdf_stream(self, stream: BinaryIO) ->
list[dict[str, Any]]:
documents.append({"text": text, "metadata": {"page_number":
page_num + 1}})
return documents
- def _parse_docx_stream(self, stream: BinaryIO) -> list[dict[str, Any]]:
+ def _parse_docx_stream(self, stream: BinaryIO, *, source_hint: str) ->
list[dict[str, Any]]:
"""
- Parse a DOCX stream into documents.
+ Parse a DOCX stream into a single document.
- Extracts paragraph text only. Tables, headers, footers, and footnotes
- are not included. For richer DOCX parsing, plug in a dedicated
- extraction tool (``Unstructured``, ``docling``) as a custom parser
- backend.
+ Paragraphs and tables in the document body are extracted in document
+ order. Each table row becomes one "| cell | cell |" line, and a nested
+ table is flattened into its cell. Headers, footers, footnotes, content
+ controls, text boxes, and pending tracked insertions are not included.
"""
try:
from docx import Document
+ from docx.table import Table
except ImportError as e:
raise AirflowOptionalProviderFeatureException(e)
doc = Document(stream)
- paragraphs = [p.text for p in doc.paragraphs if p.text.strip()]
- text = "\n\n".join(paragraphs)
+ blocks = []
+ for block in doc.iter_inner_content():
+ if isinstance(block, Table):
+ try:
+ rows = self._get_docx_table_rows(block, Table)
+ except (ValueError, RecursionError) as e:
Review Comment:
A table element missing a required `w:val`, such as `<w:gridSpan/>`, makes
python-docx raise `docx.oxml.exceptions.InvalidXmlError`. That subclasses
`XmlchemyError` and `Exception`, not `ValueError` (nor `PythonDocxError`), so
the whole file still fails. I reproduced it on 1.1.2 and 1.2.0. Word doesn't
write this, so it's the same malformed-XML case as the first-row `vMerge`, and
adding `InvalidXmlError` from `docx.oxml.exceptions` to this tuple would cover
it.
##########
providers/common/ai/src/airflow/providers/common/ai/operators/document_loader.py:
##########
@@ -450,21 +453,68 @@ def _parse_pdf_stream(self, stream: BinaryIO) ->
list[dict[str, Any]]:
documents.append({"text": text, "metadata": {"page_number":
page_num + 1}})
return documents
- def _parse_docx_stream(self, stream: BinaryIO) -> list[dict[str, Any]]:
+ def _parse_docx_stream(self, stream: BinaryIO, *, source_hint: str) ->
list[dict[str, Any]]:
"""
- Parse a DOCX stream into documents.
+ Parse a DOCX stream into a single document.
- Extracts paragraph text only. Tables, headers, footers, and footnotes
- are not included. For richer DOCX parsing, plug in a dedicated
- extraction tool (``Unstructured``, ``docling``) as a custom parser
- backend.
+ Paragraphs and tables in the document body are extracted in document
+ order. Each table row becomes one "| cell | cell |" line, and a nested
+ table is flattened into its cell. Headers, footers, footnotes, content
+ controls, text boxes, and pending tracked insertions are not included.
"""
try:
from docx import Document
+ from docx.table import Table
except ImportError as e:
raise AirflowOptionalProviderFeatureException(e)
doc = Document(stream)
- paragraphs = [p.text for p in doc.paragraphs if p.text.strip()]
- text = "\n\n".join(paragraphs)
+ blocks = []
+ for block in doc.iter_inner_content():
+ if isinstance(block, Table):
+ try:
+ rows = self._get_docx_table_rows(block, Table)
+ except (ValueError, RecursionError) as e:
+ # python-docx walks vertical merges up recursively: a
malformed merge raises
+ # ValueError and a merge spanning about 1000 rows raises
RecursionError.
+ self.log.warning(
+ "Skipping a table in %s that python-docx could not
read: %r", source_hint, e
+ )
+ continue
+ text = "\n".join(f"| {' | '.join(cells)} |" for cells in rows)
+ else:
+ text = block.text
+ if text.strip():
+ blocks.append(text)
+ text = "\n\n".join(blocks)
return [{"text": text, "metadata": {}}]
+
+ def _get_docx_table_rows(self, table: Table, table_cls: type[Table]) ->
list[list[str]]:
+ rows = []
+ for row in table.rows:
+ cells: list[str] = [""] * row.grid_cols_before
+ previous_cell = None
+ for cell in row.cells:
+ # python-docx repeats the same _Cell object for every grid
column a
+ # horizontal merge spans. A vertical merge repeats on each row
it spans.
+ if cell is previous_cell:
Review Comment:
Rows get padded for `gridBefore`/`gridAfter` so values stay under their
columns, but a cell merged across columns drops the columns it spans, so
everything to its right moves left. With `Region | Q1 | Q2` and "Total"
spanning the first two, the row comes out as `| Total | 9 |` and 9 reads as Q1.
The merged-cell test shows it too: "Note" sits over the second column.
Appending `""` for each repeated cell before the `continue` keeps "appears
once" and gives `| Total | | 9 |`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
