Prototype: Scrape `E_0003` from docx by hf-kklein · Pull Request #2 · Hochfrequenz/ebdamame

hf-kklein · 2022-12-13T07:24:23Z

No description provided.

lord-haffi · 2022-12-13T11:14:05Z

+        yield _Cell(table_column, docx_table_row.table)
+
+
+_subsequent_step_pattern = re.compile(r"^(?P<bool>(?:ja)|(?:nein))\s*(?P<subsequent_step_number>(?:\d)|ende)?")


Müsste es nicht eher

Suggested change

_subsequent_step_pattern = re.compile(r"^(?P<bool>(?:ja)|(?:nein))\s*(?P<subsequent_step_number>(?:\d)|ende)?")

_subsequent_step_pattern = re.compile(r"^(?P<bool>(?:ja)|(?:nein))\s*(?P<subsequent_step_number>(?:\d+\*?)|Ende)?")

sein? Zumindest hattest du (?:\d+\*?)|(Ende) als regex Validator in den EbdTable Feldern für die subsequent step number angegeben.
Edit: Hat der vorher beim ende überhaupt gematched? Der regex ist ja case-sensitive, wenn ich nicht irre und in den Tabellen ist Ende immer groß geschrieben?

Hat der vorher beim ende überhaupt gematched?

er matched weiter unten gegen cell.text.lower() deswegen klappt das

<-- jetzt dokumentiert

lord-haffi · 2022-12-13T11:19:53Z

+    if subsequent_step_number == "ende":
+        subsequent_step_number = "Ende"


Wenn ich das richtig sehe, ist Ende in den Tabellen immer groß geschrieben. Hab den regex oben angepasst, die Zeilen hier müssten also obsolet sein, oder?

Suggested change

if subsequent_step_number == "ende":

subsequent_step_number = "Ende"

der regex verwendet lowercase weil ich nicht ausschließen will, dass die leute manchmal "Ja" und manchmal "ja" schreiben. aber ich sehe, dass ein erklärender kommentar gut wäre.

erkläreneder kommentar: 74f1988

lord-haffi · 2022-12-13T11:27:22Z

+        self._column_index_result_code: int
+        self._column_index_note: int
+        self._row_index_last_header: Literal[0, 1]  # either 0  or 1
+        for row_index in range(0, 2):  # just check the first two rows in the constructor


Hier wäre vielleicht ein Kommentar ganz nett, dass es sich dabei um die Kopfzeilen der Tabelle handelt.

lord-haffi · 2022-12-13T11:33:35Z

+                    break  # because the prüfende rolle is always a full row with identical column cells
+                if table_cell.text == "Nr.":
+                    self._column_index_step_number = column_index
+                    self._row_index_last_header = row_index  # type:ignore[assignment]


In welchem Fall kann _row_index_last_header denn 0 sein?

wenn die erste row der tabelle nicht die Prüfende Rolle enthält: c606c61

lord-haffi · 2022-12-13T11:37:54Z

+    A class for tests of the entire package/library
+    """
+
+    @pytest.mark.datafiles("./test_data/ebd20221128.docx")


Suggested change

@pytest.mark.datafiles("./test_data/ebd20221128.docx")

@pytest.mark.datafiles("./test_data")

Wenn ich deine get_document Methode richtig verstanden habe, müsstest du den Dateinamen beim ersten Argument immer weglassen, oder?

aber der datafiles marker braucht die angabe des vollen dateinamens. Die dort genannten Dateien werden dann in ein temporäres Verzeichnis kopiert, auf das man im code dann über datafiles/<dateiname> zugreifen kann.

hf-krechan · 2022-12-13T07:38:07Z

+    with open(docx_file_path, "rb") as docx_file:
+        source_stream = BytesIO(docx_file.read())
+        # switched from StringIO to BytesIO because of:
+        # UnicodeDecodeError: 'charmap' codec can't decode byte 0x81 in position 605: character maps to <undefined>


ich gehe davon aus, dass du das encoding bei open angegeben hast oder?

open('readme.txt', encoding="utf-8")

hf-krechan · 2022-12-13T07:40:23Z

+    opens and returns the document specified in the docx_file_path using python-docx
+    """
+    # https://python-docx.readthedocs.io/en/latest/user/documents.html#opening-a-file-like-document
+    with open(docx_file_path, "rb") as docx_file:


müsste man noch den Fall abfangen, dass die Datei nicht da ist?

from pathlib import Path p = Path.home() print(p) print(p.exists())

Ich muss den fall nicht abfangen. Es ist ok, wenn er mit einem FileNotFoundError stirbt. Den Fehler zu kaschieren bringt ja auch nichts

joa, man könnte ihn mit einer schöneren Fehlermeldung aussteigen lassen. Aber ja, passt für mich

hf-krechan · 2022-12-13T07:41:48Z

@@ -1,3 +1,67 @@
 """
-src contains all your business logic
+Contains high level functions to process .docx files


Sind die Funktionen hier in der __init__.py sinnvoll hinterlegt?
Bin mir selbst nie so ganz sicher, was man am besten in eine init Datei schreibt.

mir auch nicht :D

hf-krechan · 2022-12-13T08:02:19Z

+            next_table_is_requested_table = paragraph.text.startswith(ebd_key)
+        if isinstance(table_or_paragraph, Table) and next_table_is_requested_table:


ich denke das übernehme ich für den kohlrahbi

hf-krechan · 2022-12-13T08:08:00Z

@@ -0,0 +1,128 @@
+"""
+This a docstring for the module.


achso, wirklich? Das hätte ich jetzt nicht erwartet ;P

hf-krechan · 2022-12-13T08:14:34Z

+
+class _EbdSubRowPosition(Enum):
+    """
+    describes the position of a subrow in the Docx Table


kannst du hier ein Beispiel anfügen?

068c381 und fbab7df

hf-krechan · 2022-12-13T12:38:25Z

+                if row_index == 0 and _is_pruefende_rolle_cell(table_cell):
+                    role = table_cell.text.split(":")[1].strip()
+                    break  # because the prüfende rolle is always a full row with identical column cells
+                if table_cell.text == "Nr.":


willst du hier extra so streng sein oder wäre ein startswith auch in Ordnung?

ich würde solange streng sein, bis es failed.

hf-krechan · 2022-12-13T12:42:01Z

+    converts docx tables to EbdTables
+    """
+
+    def __init__(self, docx_table: Table, ebd_key: str, chapter: str, sub_chapter: str):


vielleicht wäre es hier sinnvoll auch eine classmethod zu verwenden, um eine Instanz von DocxTableConverter zu erstellen, die etwas unabhängiger ist von dem Datenmodell.
Vergleiche https://github.com/Hochfrequenz/kohlrahbi/blob/53cf14c10cf1b965281dd705ce52126d9a3f3f50/src/kohlrahbi/helper/elixir.py#L48-L52

@hf-krechan kannst du dafür einen eigenen PR aufmachen? Ich sehe gerade den Nutzen noch nicht.

hf-krechan · 2022-12-13T12:43:51Z

+                sub_rows = []
+                step_number = row_cells[self._column_index_step_number].text.strip()
+                description = row_cells[self._column_index_description].text.strip()
+            boolean_outcome, subsequent_step_number = _cell_to_bool(row_cells[self._column_index_check_result])


wenn du oben einen besseren Namen für den boolean hast, wäre er hier auch angebracht ;)

findest du boolean_outcome noch schlecht? oder soll ich es besser "ja"/"nein" nennen? mir fällt kein besserer name ein.

hf-krechan · 2022-12-13T12:47:29Z

@@ -0,0 +1,10 @@
+# Test Data (.docx)


das ist natürlich nun sehr praktisch, dass du nur spezifische EBDs aus den docx ziehst.
mmmh ich überlege mal ob mir das auch gelingt im Kohlrahbi 👍

Co-authored-by: Leon Haffmans <49658102+lord-haffi@users.noreply.github.com>

lord-haffi · 2022-12-13T15:38:56Z

+                    self._column_index_result_code = column_index
+                elif table_cell.text == "Hinweis":
+                    self._column_index_note = column_index
+        self._metadata = EbdTableMetaData(ebd_code=ebd_key, sub_chapter=sub_chapter, chapter=chapter, role=role)


wenn die erste row der tabelle nicht die Prüfende Rolle enthält: c606c61

In dem Fall ist die Variable role aber dann nicht gesetzt, oder? Der müsste hier ja dann eigentlich crashen, testest du den Fall ab?

Ja das crasht dann. Fände ich aber ok, dann muss man sehen was der Grund ist.

Ich verstehe deinen Punkt: der Code erweckt den Eindruck er sei ganz flexibel und auf alle eventualitäten vorbereitet aber tatsächlich kann er nur den einen aktuell abgetesteten Fall handlen.

In dem Fall ist die Variable role aber dann nicht gesetzt, oder?

#9 da isser, der fall :)

lord-haffi · 2022-12-13T15:40:37Z

Der Rest lgtm

hf-kklein added 2 commits December 12, 2022 11:09

➕ Install python-docx

69d7374

Extract EBD Table from Docx (E_0003 works)

42b5215

hf-kklein self-assigned this Dec 13, 2022

hf-kklein added 5 commits December 13, 2022 08:26

so?

c60f346

so?

8285e89

so?

c94ceb3

so?

62c4805

so

398f427

hf-kklein requested review from hf-krechan and lord-haffi December 13, 2022 07:31

lord-haffi reviewed Dec 13, 2022

View reviewed changes

Comment thread src/ebddocx2table/__init__.py Outdated

lord-haffi reviewed Dec 13, 2022

View reviewed changes

Comment thread src/ebddocx2table/docxtableconverter.py

lord-haffi reviewed Dec 13, 2022

View reviewed changes

hf-krechan approved these changes Dec 13, 2022

View reviewed changes

hf-kklein and others added 9 commits December 13, 2022 14:34

Update src/ebddocx2table/__init__.py

ab00f7b

Co-authored-by: Leon Haffmans <49658102+lord-haffi@users.noreply.github.com>

allow \d+\*? in step number cells

97db754

rename and document boolean/stepnumber function

74f1988

comment on header rows

2fe575d

comment on row index

c606c61

comment on encoding error

d607ed1

module docstring

dea5016

extend docstring on helper enum

068c381

Add example

fbab7df

hf-kklein mentioned this pull request Dec 13, 2022

vielleicht wäre es hier sinnvoll auch eine classmethod zu verwenden, um eine Instanz von DocxTableConverter zu erstellen, die etwas unabhängiger ist von dem Datenmodell. #3

Closed

hf-kklein added 2 commits December 13, 2022 16:16

black

b05e123

fix pathes and CWD confusion

1352212

so vllt?

1b6899b

hf-kklein requested a review from lord-haffi December 13, 2022 15:22

lord-haffi reviewed Dec 13, 2022

View reviewed changes

hf-kklein merged commit 24d4ef1 into main Dec 13, 2022

hf-kklein deleted the first_test branch December 13, 2022 15:55

		yield _Cell(table_column, docx_table_row.table)


		_subsequent_step_pattern = re.compile(r"^(?P<bool>(?:ja)\|(?:nein))\s*(?P<subsequent_step_number>(?:\d)\|ende)?")

		if subsequent_step_number == "ende":
		subsequent_step_number = "Ende"

	@pytest.mark.datafiles("./test_data/ebd20221128.docx")
	@pytest.mark.datafiles("./test_data")

		next_table_is_requested_table = paragraph.text.startswith(ebd_key)
		if isinstance(table_or_paragraph, Table) and next_table_is_requested_table:

Conversation

hf-kklein commented Dec 13, 2022

Uh oh!

Uh oh!

lord-haffi Dec 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lord-haffi Dec 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lord-haffi Dec 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

lord-haffi Dec 13, 2022 •

edited

Loading

lord-haffi Dec 13, 2022 •

edited

Loading

lord-haffi Dec 13, 2022 •

edited

Loading