Consolidate the implementations of reserved breaks

This commit creates a new "add_reserve_teams" method and removes the
"create_or_update" remark default and the reserve teams in compute_break
as having the same effect.

It also consolidates the generated DB migrations, and removes the
extraneous "REMARK_" prefix on Remark enum values.

tabbycat/api/tests/test_views.py

@@ -136,6 +136,6 @@ def test_remove_breaking_team(self):
         self.client.login(username="admin", password="admin")
         response = self.client.patch(reverse('api-breakcategory-break', kwargs={'tournament_slug': self.tournament.slug, 'pk': 1}), {
             'team': 'http://testserver/api/v1/tournaments/demo/teams/7',
-            'remark': BreakingTeam.Remark.REMARK_WITHDRAWN,
+            'remark': BreakingTeam.Remark.WITHDRAWN,
         }, content_type='application/json')
         self.assertEqual(len(response.data), 16)

tabbycat/breakqual/aida.py

@@ -48,7 +48,7 @@ def exclude_capped_teams(self):
 
         for tsi in self.capped_teams:
             self.eligible_teams.remove(tsi)
-            self.excluded_teams[tsi] = BreakingTeam.Remark.REMARK_CAPPED
+            self.excluded_teams[tsi] = BreakingTeam.Remark.CAPPED
 
 
 class BaseAida2016BreakGenerator(BaseAidaBreakGenerator):

tabbycat/breakqual/base.py

@@ -4,6 +4,7 @@
 import logging
 from itertools import groupby
 
+from django.db.models import Q
 from django.utils.encoding import force_str
 from django.utils.translation import ngettext
 
@@ -70,6 +71,7 @@ def generate(self):
         self.retrieve_standings()
         self.filter_eligible_teams()
         self.compute_break()
+        self.add_reserve_teams()
         self.populate_database()
 
     def check_required_metrics(self, metrics):
@@ -145,9 +147,9 @@ def filter_eligible_teams(self):
         existing_remark_teams = self.team_queryset.filter(
             breakingteam__break_category=self.category,
             breakingteam__remark__isnull=False,
-        ).exclude(breakingteam__remark__exact='')
+        ).exclude(Q(breakingteam__remark__exact='') | Q(breakingteam__remark=BreakingTeam.Remark.RESERVE))
         different_break_teams = self.team_queryset.exclude(
-            breakingteam__remark=BreakingTeam.Remark.REMARK_INELIGIBLE,
+            breakingteam__remark__in=[BreakingTeam.Remark.INELIGIBLE, BreakingTeam.Remark.RESERVE],
             breakingteam__break_category__priority__gt=self.category.priority,
         ).filter(
             breakingteam__break_category__priority__gt=self.category.priority,
@@ -163,10 +165,10 @@ def filter_eligible_teams(self):
                 self.excluded_teams[tsi] = None
             elif tsi.team in ineligible_teams:
                 logger.debug("Excluding %s because it is ineligible", tsi.team)
-                self.excluded_teams[tsi] = BreakingTeam.Remark.REMARK_INELIGIBLE
+                self.excluded_teams[tsi] = BreakingTeam.Remark.INELIGIBLE
             elif tsi.team in different_break_teams:
                 logger.debug("Excluding %s because it broke in a different break", tsi.team)
-                self.excluded_teams[tsi] = BreakingTeam.Remark.REMARK_DIFFERENT_BREAK
+                self.excluded_teams[tsi] = BreakingTeam.Remark.DIFFERENT_BREAK
             else:
                 self.eligible_teams.append(tsi)
 
@@ -198,50 +200,20 @@ def compute_break(self):
         """
         raise NotImplementedError("Subclasses must implement compute_break()")
 
-    def exclude_reserve_teams(self):
+    def add_reserve_teams(self):
         # counts the number of ranks we have encountered
         # we will mark the last `self.reserve_size` ranks as participants
-        self.reserve_teams = []
+        number_to_add = self.category.break_size + self.category.reserve_size - len(self.breaking_teams)
 
-        needle = -1
-
-        # rank of the last team
-        current_rank = self.breaking_teams[-1].get_ranking("rank")
-        ranks_added = 1
-
-        while True:
-            # check if current team has the same rank as the previous one
-            tsi = self.breaking_teams[needle]
-            rank_of_team = tsi.get_ranking("rank")
-            if rank_of_team == current_rank:
-                self.reserve_teams.append(tsi)
-                needle -= 1
+        last_rank = self.breaking_teams[-1].get_ranking("rank")
+        for tsi in self.eligible_teams[self.break_size:]:
+            if (cur_rank := tsi.get_ranking("rank")) < last_rank:
+                last_rank = cur_rank
                 continue
-
-            assert rank_of_team < current_rank
-
-            # once we have added all the ranks in which some teams could be
-            # reserve teams, finish
-            if self.reserve_size <= ranks_added:
-                break
-
-            assert ranks_added < self.reserve_size
-
-            # in this case we have still not added sufficient ranks to have all
-            # the reserve teams, so add the next rank
-            current_rank = rank_of_team
-            self.reserve_teams.append(tsi)
-            logger.info(f"Reserve team {tsi.team}")
-            needle -= 1
-            ranks_added += 1
-
-            # once we have enough teams to fill the reserve we are done
-            if abs(needle) > self.reserve_size:
+            if number_to_add <= 0 and cur_rank != last_rank:
                 break
-
-        for tsi in self.reserve_teams:
-            self.breaking_teams.remove(tsi)
-            self.excluded_teams[tsi] = BreakingTeam.Remark.REMARK_RESERVE
+            self.excluded_teams[tsi] = BreakingTeam.Remark.RESERVE
+            number_to_add -= 1
 
     def populate_database(self):
         """Populates the database with BreakingTeam instances for each team
@@ -266,11 +238,7 @@ def populate_database(self):
                     defaults={
                         "rank": rank,
                         "break_rank": break_rank,
-                        "remark": (
-                            BreakingTeam.Remark.REMARK_RESERVE
-                            if self.break_size < rank <= self.reserve_size + self.break_size
-                            else None
-                        ),
+                        "remark": None,
                     },
                 )
                 bts_to_keep.append(bt.id)
@@ -290,7 +258,7 @@ def populate_database(self):
             rank = tsi.get_ranking("rank")
             # show all teams who would have broken were they not excluded as well as all the reserve
             # teams
-            if rank < self.hide_excluded_teams_from or remark == BreakingTeam.Remark.REMARK_RESERVE:
+            if rank < self.hide_excluded_teams_from or remark is BreakingTeam.Remark.RESERVE:
                 defaults = {'rank': tsi.get_ranking("rank"), 'break_rank': None}
                 if remark is not None:
                     defaults['remark'] = remark
@@ -320,11 +288,3 @@ def compute_break(self):
                     break
                 self.breaking_teams.append(tsi)
                 tied_teams_count += 1
-
-        # Then add the reserve teams (these are later marked as "reserve" in the
-        # populate database function)
-        reserve = self.eligible_teams[
-            self.break_size + tied_teams_count : self.break_size + self.reserve_size
-        ]
-        self.breaking_teams.extend(reserve)
-        assert len(self.breaking_teams) == self.reserve_size + self.break_size

tabbycat/breakqual/migrations/0007_breakcategory_reserve_size_alter_breakingteam_remark.py

@@ -1,24 +1,43 @@
-# Generated by Django 5.0.4 on 2024-09-06 17:20
+# Generated by Django 5.0.6 on 2024-09-13 02:43
 
-import django.core.validators
 from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
 
     dependencies = [
-        ('breakqual', '0006_alter_breakcategory_unique_together_and_more'),
+        ("breakqual", "0006_alter_breakcategory_unique_together_and_more"),
     ]
 
     operations = [
         migrations.AddField(
-            model_name='breakcategory',
-            name='reserve_size',
-            field=models.IntegerField(default=0, help_text='Number of reserve teams in this category.', validators=[django.core.validators.MinValueValidator(0)], verbose_name='Reserve size'),
+            model_name="breakcategory",
+            name="reserve_size",
+            field=models.PositiveIntegerField(
+                default=0,
+                help_text="Number of reserve teams in this category.",
+                verbose_name="Reserve size",
+            ),
         ),
         migrations.AlterField(
-            model_name='breakingteam',
-            name='remark',
-            field=models.CharField(blank=True, choices=[('C', 'Capped'), ('I', 'Ineligible'), ('D', 'Different break'), ('d', 'Disqualified'), ('t', 'Lost coin toss'), ('w', 'Withdrawn'), ('R', 'Reserve')], help_text="Used to explain why an otherwise-qualified team didn't break", max_length=1, null=True, verbose_name='remark'),
+            model_name="breakingteam",
+            name="remark",
+            field=models.CharField(
+                blank=True,
+                choices=[
+                    ("C", "Capped"),
+                    ("I", "Ineligible"),
+                    ("D", "Different break"),
+                    ("d", "Disqualified"),
+                    ("t", "Lost coin toss"),
+                    ("w", "Withdrawn"),
+                    ("R", "Reserve"),
+                    ("A", "Absent"),
+                ],
+                help_text="Used to explain why an otherwise-qualified team didn't break",
+                max_length=1,
+                null=True,
+                verbose_name="remark",
+            ),
         ),
     ]

tabbycat/breakqual/models.py

@@ -88,17 +88,18 @@ class BreakingTeam(models.Model):
         verbose_name=_("break rank"))
 
     class Remark(models.TextChoices):
-        REMARK_CAPPED = 'C', _("Capped")
-        REMARK_INELIGIBLE = 'I', _("Ineligible")
-        REMARK_DIFFERENT_BREAK = 'D', _("Different break")
-        REMARK_DISQUALIFIED = 'd', _("Disqualified")
-        REMARK_LOST_COIN_TOSS = 't', _("Lost coin toss")
-        REMARK_WITHDRAWN = 'w', _("Withdrawn")
-        REMARK_RESERVE = 'R', _("Reserve")
-        REMARK_ABSENT = 'A', _("Absent")
-    remark = models.CharField(max_length=1, choices=Remark, blank=True, null=True,
-                              verbose_name=_("remark"),
-                              help_text=_("Used to explain why an otherwise-qualified team didn't break"))
+        CAPPED = 'C', _("Capped")
+        INELIGIBLE = 'I', _("Ineligible")
+        DIFFERENT_BREAK = 'D', _("Different break")
+        DISQUALIFIED = 'd', _("Disqualified")
+        LOST_COIN_TOSS = 't', _("Lost coin toss")
+        WITHDRAWN = 'w', _("Withdrawn")
+        RESERVE = 'R', _("Reserve")
+        ABSENT = 'A', _("Absent")
+
+    remark = models.CharField(max_length=1, choices=Remark.choices, blank=True, null=True,
+        verbose_name=_("remark"),
+        help_text=_("Used to explain why an otherwise-qualified team didn't break"))
 
     class Meta:
         constraints = [UniqueConstraint(fields=['break_category', 'team'])]