✨Support tables spanning over multiple pages#7
Conversation
fun fun fun
hf-kklein
changed the title
| next_table_is_requested_table: bool = False | ||
| for table_or_paragraph in _get_tables_and_paragaphs(document): | ||
| tables: List[Table] = [] | ||
| tables_and_paragraphs = _get_tables_and_paragaphs(document) |
There was a problem hiding this comment.
habe den aus der loop rausgezogen, damit wir weiter unten weiter drüber loopen können (an der stelle wo wir die erste treffende tabelle gefunden haben.
|
|
||
| def convert_docx_table_to_ebd_table(self) -> EbdTable: | ||
| def _handle_single_table( | ||
| self, table: Table, row_offset: int, rows: List[EbdTableRow], sub_rows: List[EbdTableSubRow] | ||
| ) -> None: | ||
| """ | ||
| Converts the raw docx table of an EBD to an EbdTable. | ||
| The latter contains the same data but in an easily accessible format that can be used to e.g. plot real graphs. | ||
| Handles a single table (out of possible multiple tables for 1 EBD). | ||
| The results are written into rows and sub_rows. Those will be modified. | ||
| """ | ||
| rows: List[EbdTableRow] = [] | ||
| sub_rows: List[EbdTableSubRow] = [] | ||
| for table_row, sub_row_position in zip( | ||
| self._docx_table.rows[self._row_index_last_header + 1 :], | ||
| table.rows[row_offset:], | ||
| cycle([_EbdSubRowPosition.UPPER, _EbdSubRowPosition.LOWER]), | ||
| ): |
There was a problem hiding this comment.
die funktion, die vorher die eine einzige tabelle gehandlet hat, ist jetzt eine private methode, die ihre ergebnisse an zwei übergebene listen fürs rows und subrows anhängt.
| result_code = row_cells[self._column_index_result_code].text.strip() | ||
| note = row_cells[self._column_index_note].text.strip() | ||
| sub_row = EbdTableSubRow( | ||
| check_result=EbdCheckResult(subsequent_step_number=subsequent_step_number, result=boolean_outcome), | ||
| result_code=result_code or None, | ||
| note=note or None, | ||
| result_code=row_cells[self._column_index_result_code].text.strip() or None, | ||
| note=row_cells[self._column_index_note].text.strip() or None, |
There was a problem hiding this comment.
pylint hat gemeckert: too-many-locals (also zu viele lokale variablen): hier ist so ein klassiker für: das gefällt zwar dem linter aber die lesbarkeit leidet vllt ein bisschen.
| offset: int = 0 | ||
| if table_index == 0: | ||
| offset = self._row_index_last_header + 1 | ||
| self._handle_single_table(table, offset, rows, sub_rows) |
There was a problem hiding this comment.
die vormals einzige konvertierungsfunktion wird jetzt hier aufgerufen.
lord-haffi
left a comment
lord-haffi
left a comment
There was a problem hiding this comment.
Insgesamt würde ich sagen lgtm. Bei get_ebd_docx_tables ist mir nur eine Sache aufgefallen:
Meiner Meinung nach, bekommst du ein Problem, wenn nach einer Tabelle direkt ein Paragraph mit Ebd-key (und natürlich darauffolgender eigener Tabelle) kommt. Hab mal kurz die PDF überflogen. An den meisten Stellen scheint ein "unnötiger" Paragraph zu folgen, bei dem es dann nicht auffällt, wenn der geskippt wird. Falls das immer so ist, gerne kommentieren. Falls nicht, müsstest du wahrscheinlich nochmal nachbessern - in dem Fall wäre ein entsprechender Testcase bestimmt auch sinnvoll.
| tables.append(table) | ||
| # Now we have to check if the EBD table spans multiple pages and _maybe_ we have to collect more tables. | ||
| # The funny thing is: Sometimes the authors create multiple tables split over multiple lines which belong | ||
| # together, sometimes they create 1 proper table that spans multiple pages. This we won't notice here. |
There was a problem hiding this comment.
Was wird hier nicht "genoticed"? Der Unterschied zwischen den beiden Fällen, die du präsentiert hast? Oder wird einer der Fälle nicht abgefangen?
Edit: In Anbetracht des Codes hierunter nehme ich an, du meinst, dass zwar beide Fälle berücksichtigt werden, aber der Unterschied nicht im Ergebnis mit gespeichert wird?
| # sometimes the authors add blank lines before they continue with the next table | ||
| continue | ||
| else: | ||
| break # because if no other table follows, we're done collecting the tables for this EBD key |
There was a problem hiding this comment.
tables_and_paragraphs ist ja ein Generator. D.h. ja dass die Liste "on the fly" generiert wird, indem für jede Iteration der Generator die entsprechende Funktion (in diesem Fall _get_tables_and_paragaphs) bis zum nächsten yield ausführt.
Wenn du jetzt den Fall hast, dass ein Paragraph auf die Tabelle folgt, wo etwas drinsteht (z.B. ein neuer Ebd-key), dann führt der zwar den break hier aus, in der outer-loop wird der dann aber geskippt oder überseh ich hier irgendwas?
There was a problem hiding this comment.
die outer loop ist uns egal, sobald wir einmal in der inner loop waren.
91297fe
| Opens the file specified in docx_file_path and returns the table that relates to the given ebd_key. | ||
| Raises an ValueError if the table was not found. |
There was a problem hiding this comment.
Den Docstring musst du noch anpassen.
Co-authored-by: Leon Haffmans <49658102+lord-haffi@users.noreply.github.com>
lord-haffi
left a comment
lord-haffi
left a comment
There was a problem hiding this comment.
Stimmt, hab ganz vergessen, dass die Methode get_ebd_docx_tables ja nur eine einzelne Ebd-Tabelle auslesen soll. Da war ich wohl 'n bissl verwirrt ^^
Passt so für mich 👍
2 participants
fun fun fun