Skip to content

Commit 37db603

Browse files
hf-kkleinKonstantin
andauthored
feat: allow "Durchgang"-row type with only 1 subrow which has None as result (#381)
Co-authored-by: Konstantin <[email protected]>
1 parent a461845 commit 37db603

File tree

3 files changed

+117
-9
lines changed

3 files changed

+117
-9
lines changed

src/rebdhuhn/graph_conversion.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,15 @@ def _convert_row_to_decision_node(row: EbdTableRow) -> DecisionNode:
7676
return DecisionNode(step_number=row.step_number, question=row.description)
7777

7878

79-
def _yes_no_edge(decision: bool, source: DecisionNode, target: EbdGraphNode) -> EbdGraphEdge:
80-
if decision:
79+
def _yes_no_transition_edge(decision: Optional[bool], source: DecisionNode, target: EbdGraphNode) -> EbdGraphEdge:
80+
if decision is None:
81+
# happens in another PR
82+
raise NotImplementedError("None not supported yet; https://github.com/Hochfrequenz/rebdhuhn/issues/380")
83+
if decision is True:
8184
return ToYesEdge(source=source, target=target, note=None)
82-
return ToNoEdge(source=source, target=target, note=None)
85+
if decision is False:
86+
return ToNoEdge(source=source, target=target, note=None)
87+
raise ValueError(f"Decision must be either True or False or None, but was {decision}")
8388

8489

8590
def get_all_nodes(table: EbdTable) -> List[EbdGraphNode]:
@@ -132,7 +137,7 @@ def get_all_edges(table: EbdTable) -> List[EbdGraphEdge]:
132137
decision_node = _convert_row_to_decision_node(row)
133138
for sub_row in row.sub_rows:
134139
if sub_row.check_result.subsequent_step_number is not None and not _is_ende_with_no_code_but_note(sub_row):
135-
edge = _yes_no_edge(
140+
edge = _yes_no_transition_edge(
136141
sub_row.check_result.result,
137142
source=decision_node,
138143
target=nodes[sub_row.check_result.subsequent_step_number],
@@ -160,7 +165,7 @@ def get_all_edges(table: EbdTable) -> List[EbdGraphEdge]:
160165
outcome_node1=outcome_nodes_duplicates[outcome_node.get_key()], outcome_node2=outcome_node
161166
)
162167

163-
edge = _yes_no_edge(
168+
edge = _yes_no_transition_edge(
164169
sub_row.check_result.result,
165170
source=decision_node,
166171
target=nodes[outcome_node.get_key()],

src/rebdhuhn/models/ebd_table.py

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,10 @@ class EbdCheckResult:
5757
5858
To model "ja": use result=True, subsequent_step_number=None
5959
To model "nein🠖2": use result=False, subsequent_step_number="2"
60+
To model "🠖110": use result=None, subsequent_step_number="110", happens e.g. in E_0594 step_number 105
6061
"""
6162

62-
result: bool = attrs.field(validator=attrs.validators.instance_of(bool))
63+
result: Optional[bool] = attrs.field(validator=attrs.validators.optional(attrs.validators.instance_of(bool)))
6364
"""
6465
Either "ja"=True or "nein"=False
6566
"""
@@ -71,6 +72,18 @@ class EbdCheckResult:
7172
Key of the following/subsequent step, e.g. '2', or '6*' or None, if there is no follow up step
7273
"""
7374

75+
@result.validator
76+
def _validate_result(self, attribute, value) -> None: # type:ignore[no-untyped-def] #pylint:disable=unused-argument
77+
# This just ensures it's a class-level validation
78+
self._validate_only_one_none()
79+
80+
def _validate_only_one_none(self) -> None:
81+
if self.result is None and self.subsequent_step_number is None:
82+
raise ValueError(
83+
# pylint:disable=line-too-long
84+
"If the result is not boolean (meaning neither 'ja' nor 'nein' but null), the subsequent step has to be set"
85+
)
86+
7487

7588
RESULT_CODE_REGEX = r"^((?:[A-Z]\d+)|(?:A\*{2})|(?:A[A-Z]\d))$"
7689

@@ -108,7 +121,7 @@ def _check_that_both_true_and_false_occur( # type:ignore[no-untyped-def]
108121
instance: EbdTableSubRow, attribute, value: List[EbdTableSubRow]
109122
) -> None:
110123
"""
111-
Check that the subrows cover both a True and a False outcome
124+
Check that the subrows cover both a True and a False outcome. e.g. Ja -> 2 AND Nein -> 3
112125
"""
113126
# We implicitly assume that the value (list) provided already has exactly two entries.
114127
# This is enforced by other validators
@@ -119,6 +132,22 @@ def _check_that_both_true_and_false_occur( # type:ignore[no-untyped-def]
119132
)
120133

121134

135+
def _check_that_neither_true_nor_false_occur( # type:ignore[no-untyped-def]
136+
instance: EbdTableSubRow, attribute, value: List[EbdTableSubRow]
137+
) -> None:
138+
"""
139+
Check that the subrows contain neither true nor false but only a reference to a future steps.
140+
E.g. MaLo Ident E_0594 step_number 105 directly refers to step 110, no ja/nein and only 1 subrow.
141+
Screenshot here: https://github.com/Hochfrequenz/rebdhuhn/issues/379
142+
"""
143+
# We implicitly assume that the value (list) provided already has exactly two entries.
144+
# This is enforced by other validators
145+
if len(value) != 1:
146+
raise ValueError("There must be exactly one subrow when the subrows are not a 'ja' or 'nein' distinction")
147+
if value[0].check_result.result is not None:
148+
raise ValueError("The subrow must not have a 'ja' or 'nein' distinction")
149+
150+
122151
_STEP_NUMBER_REGEX = r"\d+\*?" #: regex used to validate step numbers, e.g. '4' or '7*'
123152

124153

@@ -142,8 +171,13 @@ class EbdTableRow:
142171
sub_rows: List[EbdTableSubRow] = attrs.field(
143172
validator=attrs.validators.deep_iterable(
144173
member_validator=attrs.validators.instance_of(EbdTableSubRow),
145-
iterable_validator=attrs.validators.and_(
146-
attrs.validators.min_len(2), attrs.validators.max_len(2), _check_that_both_true_and_false_occur
174+
iterable_validator=attrs.validators.or_(
175+
attrs.validators.and_(
176+
attrs.validators.min_len(2), attrs.validators.max_len(2), _check_that_both_true_and_false_occur
177+
),
178+
attrs.validators.and_(
179+
attrs.validators.min_len(1), attrs.validators.max_len(1), _check_that_neither_true_nor_false_occur
180+
),
147181
),
148182
),
149183
)

unittests/test_ebd_table_models.py

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,75 @@ def test_answer_code_astersik(self) -> None:
158158
assert isinstance(sub_row, EbdTableSubRow)
159159
assert sub_row.result_code == "A**"
160160

161+
def test_subrow_with_only_subsequent_step(self) -> None:
162+
"""
163+
This is an example from E_0594 where there is no result code but only a subsequent step.
164+
"""
165+
sub_row = EbdTableSubRow(
166+
result_code=None,
167+
check_result=EbdCheckResult(result=None, subsequent_step_number="110"),
168+
note="Aufnahme von 0..n Treffern in die Trefferliste auf Basis eines Kriteriums",
169+
)
170+
assert isinstance(sub_row, EbdTableSubRow)
171+
172+
def test_subrow_not_both_must_be_null(self) -> None:
173+
"""
174+
Check that the result code and the subsequent step are not both null.
175+
"""
176+
with pytest.raises(ValueError):
177+
_ = (EbdCheckResult(result=None, subsequent_step_number=None),)
178+
179+
def test_row_with_only_one_subrow(self) -> None:
180+
"""
181+
This is an example from E_0594 where there is a step with only 1 subrow and no ja/nein distinction
182+
"""
183+
row = EbdTableRow(
184+
step_number="105",
185+
# pylint:disable=line-too-long
186+
description="[Adressprüfung] (Straße und PLZ oder geographische Koordinaten oder Flurstücknummer) Hinweis: Das Weglassen der Hausnummer, trägt dem Umstand Rechnung, dass einige NB eher die Adresse des Geräts und nicht die Lieferadresse hinterlegt haben. Wenn der Zähler im Doppelhaus nebenan zu finden ist, weiß der Kunde das ggf.",
187+
sub_rows=[
188+
EbdTableSubRow(
189+
result_code=None,
190+
check_result=EbdCheckResult(result=None, subsequent_step_number="110"),
191+
note="Aufnahme von 0..n Treffern in die Trefferliste auf Basis eines Kriteriums",
192+
)
193+
],
194+
)
195+
assert isinstance(row, EbdTableRow)
196+
197+
def test_row_with_only_one_subrow_is_not_allowed_if_ja_nein_distinction(self) -> None:
198+
with pytest.raises(ValueError):
199+
_ = EbdTableRow(
200+
step_number="105",
201+
description="[Adressprüfung] (Straße und PLZ oder geographische Koordinaten oder Flurstücknummer) Hinweis: Das Weglassen der Hausnummer, trägt dem Umstand Rechnung, dass einige NB eher die Adresse des Geräts und nicht die Lieferadresse hinterlegt haben. Wenn der Zähler im Doppelhaus nebenan zu finden ist, weiß der Kunde das ggf.",
202+
sub_rows=[
203+
EbdTableSubRow(
204+
result_code=None,
205+
check_result=EbdCheckResult(result=True, subsequent_step_number="110"), # <--not allowed
206+
note="Aufnahme von 0..n Treffern in die Trefferliste auf Basis eines Kriteriums",
207+
)
208+
],
209+
)
210+
211+
def test_row_with_two_subrows_does_not_allow_none_result(self) -> None:
212+
with pytest.raises(ValueError):
213+
_ = EbdTableRow(
214+
step_number="105",
215+
description="xyz.",
216+
sub_rows=[
217+
EbdTableSubRow(
218+
result_code=None,
219+
check_result=EbdCheckResult(result=True, subsequent_step_number="110"),
220+
note="Foo",
221+
),
222+
EbdTableSubRow(
223+
result_code=None,
224+
check_result=EbdCheckResult(result=None, subsequent_step_number="111"), # <--not allowed
225+
note="Bar",
226+
),
227+
],
228+
)
229+
161230
def test_2023_answer_code_regex(self) -> None:
162231
"""
163232
This is an example from E_0406.

0 commit comments

Comments
 (0)