✨Implement Packages #72

hf-kklein · 2021-10-26T14:21:34Z

fixes #23

What it does

Introduces PackageResolver: The base class of all classes that translat a package key (f.e. '123P'->"[1] U ([2] O [3]) to a condition expression (using the PackageKeyConditionExpressionMapping)
Introduces hardcoded/dictbased Package Resolver

̶W̶h̶a̶t̶'̶s̶ ̶s̶t̶i̶l̶l̶ ̶m̶i̶s̶s̶i̶n̶g̶:

s̶o̶ ̶f̶a̶r̶ ̶w̶e̶ ̶m̶i̶s̶s̶ ̶t̶h̶e̶ ̶p̶o̶s̶s̶i̶b̶i̶l̶i̶t̶y̶ ̶t̶o̶ ̶e̶v̶a̶l̶u̶a̶t̶e̶ ̶a̶ ̶p̶a̶c̶k̶a̶g̶e̶ ̶a̶s̶ ̶a̶ ̶w̶h̶o̶l̶e̶/̶s̶h̶o̶r̶t̶c̶u̶t̶ ̶(̶f̶.̶e̶.̶ ̶"̶1̶2̶3̶P̶"̶-̶>̶F̶u̶l̶f̶i̶l̶l̶e̶d̶)̶ => we won't build this.

use correct return type (not consistent yet)

linter green

hf-aschloegl

Die Test schauen soweit ja gut aus, aber ich hatte doch ein paar Schwierigkeiten beim Nachvollziehen. Nachdem die Packages ja wieder ein größerer Baustein sind, könntest du die README und das Ablaufdiagramm anpassen, damit man die Idee dahinter nachvollziehen kann? Mir ist der gedachte Ablauf noch nicht hundertpro klar.
Besonders den einen Punkt den ich auch im Code angemerkt habe, wieso das eine Attribut sowohl die resolved packages, auch auch ihr evaluation result beinhalten sollte.
Vielleicht liegt das auch an der Idee des Shortcuts das Paket auszuwerten ohne es aufzulösen, aber das kannst du ja auch schon im Ablauf beschreiben ;)

Zweiter Punkt, war es Absicht nur die Pakete in der [INT P] Form zu behandeln? Weil oft ist ja auch noch die Wiederholung hinten dran a la [3P0..1].
Und können wir die Grammatik nicht direkt benutzen die Pakete als Pakete zu definieren und weiterzubenutzen, statt später nochmal mit regex oder ähnlichem wieder rauszusammeln?

hf-aschloegl · 2022-01-27T07:31:33Z

src/ahbicht/content_evaluation/categorized_key_extract.py

@@ -101,6 +123,7 @@ def generate_possible_content_evaluation_results(self) -> List[ContentEvaluation
                    if fc_kvp[0] != "fc_dummy"
                },
                requirement_constraints={rc_kvp[0]: rc_kvp[1] for rc_kvp in fc_rc_tuple[1] if rc_kvp[0] != "rc_dummy"},
+                packages={},


Ist das auch erstmal nur ein Dummy? Falls ja, kannst du dazu einen kurzen Kommentar schreiben?

kommentar ist da.

hf-aschloegl · 2022-01-27T07:36:57Z

src/ahbicht/content_evaluation/content_evaluation_result.py

+    maps the key of a package (e.g. '123') to the respective expression (e.g. '[1] U ([2] O [3])'
+    OR to a condition fulfilled value that applies to the entire package.


Das fühlt sich komisch an entweder die resolved expressionvom package da reinzuschreiben oder den condition fulfilled value.
Kannst du mir die beiden Fälle erklären, wann man welchen braucht und warum sie im selben Attribut sein sollen?

ja, ich dachte s wäre praktisch. aber es ist in der tat schwieriger zu erklären, warum wir das wollen. und

if it's hard to explain...

habe das ausgebaut.

src/ahbicht/expressions/base_transformer.py

hf-aschloegl · 2022-01-27T07:43:14Z

src/ahbicht/expressions/condition_expression_parser.py

                | condition_key
    ?brackets: "(" expression ")"
+    package: "[" INT "P]"


Damit fängst du aber noch nicht die Pakete ab, bei denen die Wiederholungen mit angegeben werden, z.B. [3P0..1]

das war mir nicht klar, dass da noch mehr kommt

ich würde die wiederholbarkeiten #96 gerne nicht in diesem PR machen.

hf-aschloegl · 2022-01-27T07:45:09Z

src/ahbicht/expressions/condition_expression_parser.py

+        # as of now the packages are extracted separatly via their rule, not by analyzing the key
+        # elif condition_node_type is ConditionNodeType.PACKAGE:
+        #    result.package_keys.append(condition_key)


Kannst du auch noch den Grund nennen, wieso?

hf-aschloegl · 2022-01-27T07:54:56Z

unittests/test_categorized_key_extraction.py

@@ -130,5 +151,5 @@ def test_possible_cer_generation_large_results(self, test_file_path, datafiles):
        categoried_keys = CategorizedKeyExtractSchema().load(file_content["categorizedKeyExtract"])
        expected_result = ContentEvaluationResultSchema(many=True).load(file_content["expected_result"])
        actual = categoried_keys.generate_possible_content_evaluation_results()
-        json_string = ContentEvaluationResultSchema(many=True).dumps(actual)
+        # json_string = ContentEvaluationResultSchema(many=True).dumps(actual)


Warum wurde das auskommentiert?

ich glaube nur, weil es nicht genutzt wurde.

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

hf-kklein · 2022-01-28T17:03:31Z

src/ahbicht/expressions/condition_expression_parser.py

@@ -90,19 +119,29 @@ def extract_categorized_keys_from_tree(
            result.hint_keys.append(condition_key)
        elif condition_node_type is ConditionNodeType.FORMAT_CONSTRAINT:
            result.format_constraint_keys.append(condition_key)
-        elif condition_node_type is ConditionNodeType.PACKAGE:
-            result.package_keys.append(condition_key)
+        # As of now the packages are extracted separately via their rule, not by analyzing the key.


und an der stelle bricht sehr viel zusammen...

hf-kklein · 2022-01-28T17:35:39Z

src/ahbicht/expressions/condition_expression_parser.py

+    There has to be a better way of doing this.
+    The rule name can either be "condition_key" or "package"
+    """
+    # Prior to the introduction of packages we could simply loop over all the 'INT' terminals and be sure that


hard to explain, ugly workaround

hf-kklein · 2022-01-28T17:37:44Z

obsoleted by #116

add package syntax to condition expression grammar

a53d6e6

hf-kklein self-assigned this Oct 26, 2021

update test

8b878b3

hf-kklein changed the title ~~Implement Packages~~ ✨Implement Packages Oct 26, 2021

hf-kklein added 2 commits October 27, 2021 07:50

wip

30770f9

Merge branch 'main' into packages

4f83b1f

hf-kklein assigned hf-aschloegl Nov 2, 2021

Merge branch 'main' into packages

7ec7ab6

hf-kklein mentioned this pull request Nov 22, 2021

✨(ConditionKey🠒ConditionText) and (PackageKey🠒ConditionExpression) mappings; ➕marshmallow_enum #85

Merged

This comment has been minimized.

Sign in to view

hf-kklein added 10 commits December 28, 2021 11:28

Merge branch 'main' into packages

5be693d

tests are green

91261a4

tests are red again ;)

cb5363e

green again

4ed82d1

even greener

1307273

ok

e0e86e5

naming

2f0f6ed

it's hard to explain

8576732

more ideas

a107207

ignore mypy

a5c762a

hf-kklein mentioned this pull request Dec 28, 2021

Implement "Packages" for EdifactFormatVersion.FV2204 (aka MaKo2022) #23

Closed

1 task

hf-kklein added 7 commits December 30, 2021 15:46

Merge branch 'main' into packages

b5bab29

wip, schlus sfür heute

6f557a5

use correct return type (not consistent yet)

tests are green

f99ad1e

linter green

remove unused code

b344b95

does this please the 3.9 test?

d7cfc6c

Merge branch 'main' into packages

0e75995

remove shortcut feature again

1b1eecb

hf-kklein marked this pull request as ready for review January 2, 2022 16:54

add the usual overhead to the package resolver

274203a

hf-kklein added 3 commits January 2, 2022 19:01

please the linter (duplicate-code)

461c05f

Merge branch 'main' into packages

9a71aa5

Merge branch 'main' into packages

72f1573

hf-kklein requested a review from a team January 21, 2022 13:10

hf-aschloegl requested changes Jan 27, 2022

View reviewed changes

hf-kklein and others added 6 commits January 27, 2022 11:16

Update src/ahbicht/expressions/base_transformer.py

a47aee6

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

black

6fd4485

fix linti binti warnings

dd15488

add todo comments regarding wiederholbarkeiten

64318c1

add comment to empty pacakges in CEE

dc2d71a

trying to explain

6120553

hf-kklein commented Jan 28, 2022

View reviewed changes

hf-kklein mentioned this pull request Jan 28, 2022

✨ Implement Packages: Resolve package nodes [...P] to sub trees using a lark transformer #116

Merged

hf-kklein closed this Jan 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨Implement Packages #72

✨Implement Packages #72

hf-kklein commented Oct 26, 2021 •

edited

Loading

This comment has been minimized.

hf-aschloegl left a comment

hf-aschloegl Jan 27, 2022

hf-kklein Jan 28, 2022

hf-aschloegl Jan 27, 2022

hf-kklein Jan 27, 2022 •

edited

Loading

hf-kklein Jan 28, 2022

hf-aschloegl Jan 27, 2022

hf-kklein Jan 27, 2022

hf-kklein Jan 28, 2022

hf-aschloegl Jan 27, 2022

hf-aschloegl Jan 27, 2022

hf-kklein Jan 27, 2022

hf-kklein Jan 28, 2022

hf-kklein Jan 28, 2022

hf-kklein commented Jan 28, 2022

		maps the key of a package (e.g. '123') to the respective expression (e.g. '[1] U ([2] O [3])'
		OR to a condition fulfilled value that applies to the entire package.

✨Implement Packages #72

✨Implement Packages #72

Conversation

hf-kklein commented Oct 26, 2021 • edited Loading

What it does

̶W̶h̶a̶t̶'̶s̶ ̶s̶t̶i̶l̶l̶ ̶m̶i̶s̶s̶i̶n̶g̶:

This comment has been minimized.

hf-aschloegl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hf-kklein Jan 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hf-kklein commented Jan 28, 2022

hf-kklein commented Oct 26, 2021 •

edited

Loading

hf-kklein Jan 27, 2022 •

edited

Loading