From b1d5e1b08f504920ec43b6942be9b83b43060a0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C4=9Bj=20T=C3=BD=C4=8D?= Date: Fri, 15 Mar 2024 16:52:10 +0100 Subject: [PATCH] Categorize problem types --- estimage/problems/__init__.py | 47 ++++++++++++++++-- estimage/solutions/__init__.py | 28 ++++++++++- estimage/webapp/main/forms.py | 28 +++++++++++ estimage/webapp/main/routes.py | 29 ++++++++++- estimage/webapp/templates/problems.html | 20 ++++---- tests/test_problems.py | 64 ++++++++++++++++++++++++- tests/test_solutions.py | 2 + 7 files changed, 200 insertions(+), 18 deletions(-) diff --git a/estimage/problems/__init__.py b/estimage/problems/__init__.py index f32beb5..f23ea06 100644 --- a/estimage/problems/__init__.py +++ b/estimage/problems/__init__.py @@ -1,5 +1,6 @@ import dataclasses import typing +import collections import numpy as np @@ -10,14 +11,17 @@ @dataclasses.dataclass(init=False) class Problem: description: str - affected_cards_names: list[str] + affected_card_name: str tags: set[str] def __init__(self): self.description = "" - self.affected_cards_names = [] + self.affected_card_name = "" self.tags = frozenset() + def format_task_name(self): + pass + def add_tag(self, tag): self.tags = frozenset(self.tags.union([tag])) @@ -45,7 +49,7 @@ def _numbers_differ_significantly(self, lhs, rhs): def _create_and_add_card_problem(self, card): problem = Problem() - problem.affected_cards_names.append(card.name) + problem.affected_card_name = card.name self.problems.append(problem) return problem @@ -58,7 +62,7 @@ def _treat_inconsistent_estimate(self, card, computed_nominal_cost, recorded_cos if computed_nominal_cost == 0: self._inconsistent_card_missing_estimates(problem, card, recorded_cost) else: - self._inconsistent_card_differing_estimate(problem, card, recorded_cost, expected_computed_cost) + self._inconsistent_card_differing_estimate(problem, card, recorded_cost, expected_computed_cost, computed_nominal_cost) def _card_has_no_children_with_children(self, card): for child in card.children: @@ -75,9 +79,11 @@ def _inconsistent_card_missing_estimates(self, problem, card, recorded_cost): if self._card_has_no_children_with_children(card): problem.add_tag("childless_children") - def _inconsistent_card_differing_estimate(self, problem, card, recorded_cost, expected_computed_cost): + def _inconsistent_card_differing_estimate(self, problem, card, recorded_cost, expected_computed_cost, computed_nominal_cost): if expected_computed_cost < recorded_cost: problem.add_tag("sum_of_children_lower") + if recorded_cost <= computed_nominal_cost: + problem.add_tag("estimate_within_nominal") problem.description = ( f"'{card.name}' has inconsistent recorded point cost of {recorded_cost:.2g}, " f"while the deduced cost is {expected_computed_cost:.2g}" @@ -98,3 +104,34 @@ def _analyze_inconsistent_record(self, card): return self._treat_inconsistent_estimate(card, computed_nominal_cost, recorded_cost, expected_computed_cost) + + +class ProblemCategory: + name = "generic" + + def matches(self, p: Problem): + return True + + +class ProblemClassifier: + def __init__(self): + self.not_classified = [] + self.classified_problems = collections.defaultdict(list) + self._categories = dict() + + def classify(self, problems: typing.Iterable[Problem]): + for p in problems: + self._classify_problem(p) + + def _classify_problem(self, problem: Problem): + for c in self._categories.values(): + if c.matches(problem): + self.classified_problems[c.name].append(problem) + return + self.not_classified.append(problem) + + def add_category_type(self, cat_type: typing.Type[ProblemCategory]): + if (name := cat_type.name) in self._categories: + msg = f"Already have a category named '{name}'" + raise KeyError(msg) + self._categories[name] = cat_type() diff --git a/estimage/solutions/__init__.py b/estimage/solutions/__init__.py index fd12f56..262fadf 100644 --- a/estimage/solutions/__init__.py +++ b/estimage/solutions/__init__.py @@ -21,14 +21,17 @@ def prime(self, cards: typing.Iterable[BaseCard]): class SolutionByUpdating(Solution): end_value: float + updates_model: bool def __init__(self): super().__init__() self.end_value = None + self.updates_model = True def describe(self): return f"Update the record of '{self.card_name}'" + class SolutionByUpdatingChildren(SolutionByUpdating): action = "update_children_points" @@ -46,6 +49,7 @@ class SolutionByUpdatingSelf(SolutionByUpdating): def __init__(self): super().__init__() + self.updates_model = False def describe(self): return f"Update the record of '{self.card_name}', so it matches records of its children." @@ -54,6 +58,11 @@ def prime(self, cards: typing.Iterable[BaseCard]): pass +class SafeSolutionByUpdatingSelf(SolutionByUpdatingSelf): + def describe(self): + return f"Update the record of '{self.card_name}', so it matches records of its children, likely reflecting their development." + + class ProblemSolver: SOLUTIONS = [] @@ -77,6 +86,21 @@ def problem_solution(func): return func +@problem_solution +def get_safe_solution_of_inconsistent_parent(problem: Problem): + if "inconsistent_estimate" not in problem.tags: + return + if "estimate_within_nominal" not in problem.tags: + return + if "sum_of_children_lower" not in problem.tags: + return + if "missing_estimates" in problem.tags: + return + ret = SafeSolutionByUpdatingSelf() + ret.card_name = problem.affected_card_name + return ret + + @problem_solution def get_solution_of_inconsistent_parent(problem: Problem): if "inconsistent_estimate" not in problem.tags: @@ -84,7 +108,7 @@ def get_solution_of_inconsistent_parent(problem: Problem): if "missing_estimates" in problem.tags: return ret = SolutionByUpdatingSelf() - ret.card_name = problem.affected_cards_names[0] + ret.card_name = problem.affected_card_name return ret @@ -98,5 +122,5 @@ def get_solution_of_inconsistent_children(problem: Problem): return ret = SolutionByUpdatingChildren() - ret.card_name = problem.affected_cards_names[0] + ret.card_name = problem.affected_card_name return ret diff --git a/estimage/webapp/main/forms.py b/estimage/webapp/main/forms.py index e8118e8..e2d5035 100644 --- a/estimage/webapp/main/forms.py +++ b/estimage/webapp/main/forms.py @@ -98,3 +98,31 @@ class PointEstimationForm(FlaskForm): most_likely = wtforms.SelectField("Most Likely", choices=FIB) pessimistic = wtforms.SelectField("Pessimistic", choices=FIB) submit = SubmitField("Save Estimate") + + +class MultiCheckboxField(wtforms.SelectMultipleField): + """ + A multiple-select, except displays a list of checkboxes. + + Iterating the field will produce subfields, allowing custom rendering of + the enclosed checkbox fields. + """ + widget = wtforms.widgets.ListWidget(prefix_label=False) + option_widget = wtforms.widgets.CheckboxInput() + + def get_selected(self): + ret = [] + for x in self: + print(11, x, 22) + + +class ProblemForm(FlaskForm): + + def add_problems(self, problems): + for p in problems: + self.problems.choices.append((p.affected_card_name, p.description)) + + problem = wtforms.HiddenField("problem") + solution = wtforms.HiddenField("solution") + problems = MultiCheckboxField("Problems", choices=[]) + submit = SubmitField("Solve Problems") diff --git a/estimage/webapp/main/routes.py b/estimage/webapp/main/routes.py index 16051fb..d1bdee9 100644 --- a/estimage/webapp/main/routes.py +++ b/estimage/webapp/main/routes.py @@ -447,6 +447,33 @@ def view_problems(): solver = solutions.ProblemSolver() sols = {p.description: solver.get_solution_of(p) for p in probs} + form = forms.ProblemForm() + form.add_problems(problem_detector.problems) + form.problem.data = "Missing Update" + form.solution.data = "Update parents" + return web_utils.render_template( 'problems.html', title='Problems', - all_cards_by_id=all_cards_by_id, problems=probs, solutions=sols) + all_cards_by_id=all_cards_by_id, problems=probs, solutions=sols, form=form) + + +@bp.route('/problems/fix', methods=['POST']) +@flask_login.login_required +def fix_problems(): + user = flask_login.current_user + user_id = user.get_id() + + all_cards_by_id, model = web_utils.get_all_tasks_by_id_and_user_model("retro", user_id) + all_cards = list(all_cards_by_id.values()) + + problem_detector = problems.ProblemDetector(model, all_cards) + + form = forms.ProblemForm() + form.add_problems(problem_detector.problems) + if form.validate_on_submit(): + print(f"Fix {form.problem.data} by {form.solution.data}") + print(f"Fix: {form.problems.data}") + else: + flask.flash(f"Error handing over solution: {form.errors}") + return flask.redirect( + web_utils.head_url_for("main.view_problems")) diff --git a/estimage/webapp/templates/problems.html b/estimage/webapp/templates/problems.html index 737300f..c09fca9 100644 --- a/estimage/webapp/templates/problems.html +++ b/estimage/webapp/templates/problems.html @@ -1,14 +1,11 @@ {% extends "general_retro.html" %} {% import "utils.j2" as utils with context %} +{% from 'bootstrap5/form.html' import render_form %} -{% macro print_name_replaced_with_links(text, names) %} -{% if names | length > 1 %} -{{ print_name_replaced_with_links(print_name_replaced_with_links(text, [names[0]]), names[1:]) }} -{% else %} -{{ text.replace(names[0], utils.task_or_epic_link(all_cards_by_id[names[0]], "retrospective")) | safe }} -{% endif %} +{% macro print_name_replaced_with_links(text, name) %} +{{ text.replace(name, utils.task_or_epic_link(all_cards_by_id[name], "retrospective")) | safe }} {% endmacro %} @@ -18,9 +15,9 @@

Problems

{% for p in problems %} -

{{ " ".join(p.affected_cards_names) }}

+

{{ p.affected_card_name }}

- {{ print_name_replaced_with_links(p.description, p.affected_cards_names) }} + {{ print_name_replaced_with_links(p.description, p.affected_card_name) }}

Tags: @@ -34,6 +31,13 @@

{{ " ".join(p.affected_cards_names) }}

{% endfor %}
+
+

Inconsistent Values of Parents

+

+ Solvable by updating parents to match children. +

+ {{ render_form(form, action=head_url_for("main.fix_problems")) }} +
{% endblock content %} diff --git a/tests/test_problems.py b/tests/test_problems.py index 3a29b97..b714631 100644 --- a/tests/test_problems.py +++ b/tests/test_problems.py @@ -66,12 +66,22 @@ def test_model_notices_basic_inconsistency(cards_one_two): assert "of 1" in problem.description -def test_model_notices_inconsistency_maybe_caused_by_progress(cards_one_two): +def test_model_notices_inconsistency_unlikely_caused_by_progress(cards_one_two): card_one, card_two = cards_one_two card_two.point_cost = 0.5 problem = get_problem_of_cards(cards_one_two) assert "inconsistent_estimate" in problem.tags assert "sum_of_children_lower" in problem.tags + assert not "estimate_within_nominal" in problem.tags + + +def test_model_notices_inconsistency_probably_caused_by_progress(cards_one_two): + card_one, card_two = cards_one_two + card_two.status = "done" + problem = get_problem_of_cards(cards_one_two) + assert "inconsistent_estimate" in problem.tags + assert "sum_of_children_lower" in problem.tags + assert "estimate_within_nominal" in problem.tags def test_model_notices_children_not_estimated(cards_one_two): @@ -99,4 +109,54 @@ def test_model_finds_status_problem(cards_one_two): card_two.point_cost = card_one.point_cost problem = get_problem_of_cards(cards_one_two) - assert "one" in problem.affected_cards_names + assert "one" == problem.affected_card_name + + +def test_problem_categories_trivial(): + dumb_classifier = tm.ProblemClassifier() + p = tm.Problem() + p.add_tag("urgent") + dumb_classifier.classify([p]) + others = dumb_classifier.not_classified + assert len(others) == 1 + assert others[0] == p + + +class CustomClassifier(tm.ProblemClassifier): + pass + + +class Underestimation(tm.ProblemCategory): + name = "underestimation" + + def matches(self, p): + return "underestimated" in p.tags + + +def test_problem_category_match(): + cat = Underestimation() + p = tm.Problem() + p.add_tag("underestimated") + assert cat.matches(p) + + bad_p = tm.Problem() + bad_p.add_tag("nothing") + assert not cat.matches(bad_p) + + +def test_problem_categories_no_duplication(): + smart_classifier = CustomClassifier() + smart_classifier.add_category_type(Underestimation) + with pytest.raises(KeyError): + smart_classifier.add_category_type(Underestimation) + + +def test_problem_categories_basic(): + smart_classifier = CustomClassifier() + smart_classifier.add_category_type(Underestimation) + p = tm.Problem() + p.add_tag("underestimated") + smart_classifier.classify([p]) + assert not smart_classifier.not_classified + urgent_problems = smart_classifier.classified_problems + assert urgent_problems["underestimation"][0] == p diff --git a/tests/test_solutions.py b/tests/test_solutions.py index 71d127b..1c0bfc4 100644 --- a/tests/test_solutions.py +++ b/tests/test_solutions.py @@ -26,6 +26,7 @@ def test_basic_inconsistency_solution(solver, cards_one_two): solution = solutions[0] assert solution.action == "update_points" assert solution.card_name == "one" + assert solution.updates_model == False solution.prime([card_one]) # assert solution.end_value == card_two.point_cost @@ -42,6 +43,7 @@ def test_update_children_inconsistency_solution(solver, cards_one_two): solution = solutions[0] assert solution.action == "update_children_points" assert solution.card_name == "one" + assert solution.updates_model == True solution.prime([card_one]) assert solution.end_value == card_one.point_cost / 2.0