Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle multiple lines correctly #28

Merged
merged 7 commits into from
Sep 23, 2022
Merged

Handle multiple lines correctly #28

merged 7 commits into from
Sep 23, 2022

Conversation

hf-krechan
Copy link
Collaborator

No description provided.

@hf-krechan hf-krechan marked this pull request as ready for review September 23, 2022 09:51
Copy link
Contributor

@hf-kklein hf-kklein left a comment

Choose a reason for hiding this comment

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

Nur ein paar anmerkungen

@@ -42,13 +42,14 @@ def parse_paragraph_in_edifact_struktur_column_to_dataframe(
elif tab_count == 1:
dataframe.at[row_index, "Segment Gruppe"] = splitted_text_at_tabs[0]
dataframe.at[row_index, "Segment"] = splitted_text_at_tabs[1]
elif tab_count == 0 and not paragraph.text == "":
if paragraph.runs[0].bold:
elif tab_count == 0 and joined_text != "":
Copy link
Contributor

Choose a reason for hiding this comment

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

müsste man joined_text hier für den empty-string vergleich nicht strippen? zwei leere paragraphen gejoined wären ja mindestens ein leerzeichen.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good point. Habe s ergänzt in 28708aa

@@ -325,10 +325,10 @@ def test_parse_paragraph_in_edifact_struktur_column_to_dataframe(

# insert text
self.test_cell.text = text_content
test_paragraph = self.test_cell.paragraphs[0]
test_paragraph = [self.test_cell.paragraphs[0]]
Copy link
Contributor

Choose a reason for hiding this comment

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

ich weiß, ich hab das uach nicht gemacht, aber ein unittest für eine mehrzeilige überschrift wäre nice.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

bin generell mit dem test setup unzufrieden da es noch recht komplex ist.
Ich nehme mal den Punkt in ein Ticket auf, damit wir es nicht vergessen.
Würde das erstmal hinten anstellen wenn es okay ist.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

#30

@hf-krechan hf-krechan merged commit ddf73a9 into rework Sep 23, 2022
@hf-krechan hf-krechan deleted the improve_multilines branch September 23, 2022 15:40
hf-krechan added a commit that referenced this pull request Sep 26, 2022
* 📝🚧 Make a small mermaid diagram to get the idea of the workflow

and add persona to concentrate on one goal first

* 🙈 Add macOS related files to gitignore

* 🚧🎨 Add Elixir class

* 🚧 working WIP

* 🔥 Delete deprecated lines

* 🚧 change ahb_file_name

* 🩹 Fix row_index counting

* 🔥 replace row

* 💡 Update docstrings

* ✅ Update unit tests

* ✅ Fix unit tests

* 🚨 Fix linter warnings

* 🔄 Update dependencies

* 🎨 Apply isort

* 🏷 Remove unnecessary type hint

Co-authored-by: konstantin <[email protected]>

* 🏷 Remove type hint

Co-authored-by: konstantin <[email protected]>

* Handle multiple lines correctly (#28)

* Handle multiple lines correctly

* 🩹 Fix issues after cherry pick

* ✅🩹 Fix broken unitttests

* ⚙️ Add setting for pytest to fix import issue

* 🚨 Fix linter warnings

* 🎨 Strip joined_text in elif conidtion

* 🎨 Improve readability for columns creation

* 🎨 improve variable name for name of base columnes

* 🩹 Fix errors after merging

* 🚨 Fix linter and ignore mypy warnings

* 🩹 Fix issue that sometimes there are empty lines with only Bedingung

* 💡 Write some explanation

* 💡 Add reason for the classmethod in Elixir

Co-authored-by: konstantin <[email protected]>
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.

2 participants