From 4109bb4f02514a73e0b5f44f1078bd81a88ed5d3 Mon Sep 17 00:00:00 2001 From: Elaye Karstadt Date: Tue, 13 Aug 2024 19:48:32 +0300 Subject: [PATCH] Remove mutable default args This is a common pitfall in Python that causes unexpected behavior. Python evaluates functions on module load and generates mutable default argument objects once, then persist them whenever the function is called. --- .gitignore | 4 ++++ tabbycat/adjfeedback/tests/test_progress.py | 9 +++++++-- tabbycat/breakqual/liveness.py | 13 +++++++++---- tabbycat/draw/generator/pairing.py | 15 ++++++++++----- tabbycat/draw/tables.py | 5 ++++- tabbycat/draw/views.py | 10 ++++++++-- tabbycat/importer/importers/base.py | 5 ++++- tabbycat/options/forms.py | 5 ++++- tabbycat/results/result.py | 4 ++-- tabbycat/tournaments/models.py | 6 +++++- tabbycat/utils/admin.py | 6 +++++- tabbycat/utils/tables.py | 4 +++- 12 files changed, 65 insertions(+), 21 deletions(-) diff --git a/.gitignore b/.gitignore index 277caf7bfe9..34a1c64c55d 100644 --- a/.gitignore +++ b/.gitignore @@ -13,6 +13,10 @@ tabbycat/static/jsi18n/ tabbycat/static/vue/ tabbycat/static/fonts/ +# IDE files +.idea/ +.vscode/ + # Translations .tx/ diff --git a/tabbycat/adjfeedback/tests/test_progress.py b/tabbycat/adjfeedback/tests/test_progress.py index 15a06fef632..b8a63a301c5 100644 --- a/tabbycat/adjfeedback/tests/test_progress.py +++ b/tabbycat/adjfeedback/tests/test_progress.py @@ -61,13 +61,15 @@ def _dt(self, debate, t): def _da(self, debate, a): return DebateAdjudicator.objects.get(debate=debate, adjudicator=self._adj(a)) - def _create_debate(self, teams, adjs, votes, trainees=[], venue=None): + def _create_debate(self, teams, adjs, votes, trainees=None, venue=None): """Enters a debate into the database, using the teams and adjudicators specified. `votes` should be a string (or iterable of characters) indicating "a" for affirmative or "n" for negative, e.g. "ann" if the chair was rolled in a decision for the negative. The method will give the winning team all 76s and the losing team all 74s. The first adjudicator is the chair; the rest are panellists.""" + if trainees is None: + trainees = [] if venue is None: venue = Venue.objects.first() debate = Debate.objects.create(round=self.rd, venue=venue) @@ -134,7 +136,10 @@ def _create_feedback(self, source, target, confirmed=True, ignored=False): # From team # ========================================================================== - def assertExpectedFromTeamTracker(self, debate, t, expected, fulfilled, count, submissions, targets, tracker_kwargs={}): # noqa + def assertExpectedFromTeamTracker(self, debate, t, expected, fulfilled, count, submissions, targets, tracker_kwargs=None): # noqa + if tracker_kwargs is None: + tracker_kwargs = {} + tracker = FeedbackExpectedSubmissionFromTeamTracker(self._dt(debate, t), **tracker_kwargs) self.assertIs(tracker.expected, expected) self.assertIs(tracker.fulfilled, fulfilled) diff --git a/tabbycat/breakqual/liveness.py b/tabbycat/breakqual/liveness.py index 1ecda6df038..2e85a33bf11 100644 --- a/tabbycat/breakqual/liveness.py +++ b/tabbycat/breakqual/liveness.py @@ -26,7 +26,9 @@ def get_coefficient(m, k): return half_row + half_row[-2::-1] -def liveness_twoteam(is_general, current_round, break_size, total_teams, total_rounds, team_scores=[]): +def liveness_twoteam(is_general, current_round, break_size, total_teams, total_rounds, team_scores=None): + if team_scores is None: + team_scores = [] if total_teams < break_size or (not is_general and len(team_scores) <= break_size): return 0, -1 # special case, everyone is safe @@ -64,7 +66,8 @@ def liveness_twoteam(is_general, current_round, break_size, total_teams, total_r return safe, dead -def liveness_bp(is_general, current_round, break_size, total_teams, total_rounds, team_scores=[]): +def liveness_bp(is_general, current_round, break_size, total_teams, total_rounds, team_scores=None): + team_scores = [] if team_scores is None else team_scores if total_teams < break_size or (not is_general and len(team_scores) <= break_size): return -1, -1 # special case, everyone is safe @@ -103,9 +106,11 @@ def liveness_bp(is_general, current_round, break_size, total_teams, total_rounds # The dead score is the highest score from which a team can no longer # 'catch' a team in the last breaking spot. + + # All are live if no team scores exist (i.e. Round 1) + dead = -1 + if len(team_scores) >= break_size - 1: dead = team_scores[break_size-1] - points_to_go - 1 - else: - dead = -1 # All are live if no team scores exist (i.e. Round 1) return safe, dead diff --git a/tabbycat/draw/generator/pairing.py b/tabbycat/draw/generator/pairing.py index 72e182dc27d..cdb4315ceed 100644 --- a/tabbycat/draw/generator/pairing.py +++ b/tabbycat/draw/generator/pairing.py @@ -37,10 +37,15 @@ class BasePairing: This is a base class for functionality common to both two-team pairings and BP pairings.""" - def __init__(self, teams, bracket, room_rank, flags=[], team_flags={}): + def __init__(self, teams, bracket, room_rank, flags=None, team_flags=None): """'teams' must be a list of two teams, or four teams if it's for BP. 'bracket' and 'room_rank' are both integers. 'flags' is a list of strings.""" + if flags is None: + flags = [] + if team_flags is None: + team_flags = {} + self.teams = list(teams) self.bracket = bracket self.room_rank = room_rank @@ -95,7 +100,7 @@ class Pairing(BasePairing): sides = [DebateSide.AFF, DebateSide.NEG] - def __init__(self, teams, bracket, room_rank, num_sides=2, flags=[], team_flags={}): + def __init__(self, teams, bracket, room_rank, num_sides=2, flags=None, team_flags=None): super().__init__(teams, bracket, room_rank, flags, team_flags) assert len(self.teams) == 2, "There must be two teams in a Pairing" @@ -147,7 +152,7 @@ class ResultPairing(Pairing): This class is the data structure expected by DrawGenerator classes, when taking information about the results of the previous round.""" - def __init__(self, teams, bracket, room_rank, flags=[], team_flags={}, winner=None): + def __init__(self, teams, bracket, room_rank, flags=None, team_flags=None, winner=None): super().__init__(teams, bracket, room_rank, flags, team_flags) self.set_winner(winner) @@ -176,7 +181,7 @@ def winner(self): class PolyPairing(BasePairing): """Pairing class for British Parliamentary and Public Speaking.""" - def __init__(self, teams, bracket, room_rank, num_sides=4, flags=[], team_flags={}): + def __init__(self, teams, bracket, room_rank, num_sides=4, flags=None, team_flags=None): super().__init__(teams, bracket, room_rank, flags, team_flags) def __repr__(self): @@ -191,7 +196,7 @@ class BPEliminationResultPairing(PolyPairing): sides = [DebateSide.OG, DebateSide.OO, DebateSide.CG, DebateSide.CO] - def __init__(self, teams, bracket, room_rank, flags=[], team_flags={}, advancing=[]): + def __init__(self, teams, bracket, room_rank, flags=None, team_flags=None, advancing=None): super().__init__(teams, bracket, room_rank, 4, flags, team_flags) self.set_advancing(advancing) diff --git a/tabbycat/draw/tables.py b/tabbycat/draw/tables.py index 9140bc3cbf8..e217f27e8d4 100644 --- a/tabbycat/draw/tables.py +++ b/tabbycat/draw/tables.py @@ -65,7 +65,10 @@ class PublicDrawTableBuilder(BaseDrawTableBuilder): def get_sides(self, debates: List['Debate']) -> int: return max([dt.side for debate in debates for dt in debate.debateteams], default=self.tournament.pref('teams_in_debate') - 1) + 1 - def add_debate_team_columns(self, debates, highlight=[]): + def add_debate_team_columns(self, debates, highlight=None): + if highlight is None: + highlight = [] + all_sides_confirmed = all(debate.sides_confirmed for debate in debates) # should already be fetched for side in range(self.get_sides(debates)): diff --git a/tabbycat/draw/views.py b/tabbycat/draw/views.py index 1038190b25c..65758afd65a 100644 --- a/tabbycat/draw/views.py +++ b/tabbycat/draw/views.py @@ -90,7 +90,10 @@ def get_page_subtitle(self): else: return "" - def populate_table(self, debates, table, highlight=[]): + def populate_table(self, debates, table, highlight=None): + if highlight is None: + highlight = [] + table.add_debate_venue_columns(debates) table.add_debate_team_columns(debates, highlight) table.add_debate_adjudicators_column(debates, show_splits=False) @@ -232,7 +235,10 @@ def get_page_subtitle(self): def get_page_emoji(self): return None - def populate_table(self, debates, table, highlight=[]): + def populate_table(self, debates, table, highlight=None): + if highlight is None: + highlight = [] + table.add_round_column(d.round for d in debates) super().populate_table(debates, table, highlight=highlight) diff --git a/tabbycat/importer/importers/base.py b/tabbycat/importer/importers/base.py index 9db9bec3799..8151017dc9b 100644 --- a/tabbycat/importer/importers/base.py +++ b/tabbycat/importer/importers/base.py @@ -25,10 +25,13 @@ def convert_bool(value): raise ValueError('Invalid boolean value: %s' % (value,)) -def make_interpreter(DELETE=[], **kwargs): # noqa: N803 +def make_interpreter(DELETE=None, **kwargs): # noqa: N803 """Convenience function for building an interpreter. The default interpreter (i.e. the one returned if no arguments are passed to this function) just removes blank values.""" + if DELETE is None: + DELETE = [] + def interpreter(lineno, line): # remove blank and unwanted values line = { diff --git a/tabbycat/options/forms.py b/tabbycat/options/forms.py index 31b2045d436..f35a9f33a34 100644 --- a/tabbycat/options/forms.py +++ b/tabbycat/options/forms.py @@ -48,7 +48,10 @@ def get_pref(name, section=section): return self.cleaned_data -def tournament_preference_form_builder(instance, preferences=[], **kwargs): +def tournament_preference_form_builder(instance, preferences=None, **kwargs): + if preferences is None: + preferences = [] + if kwargs.get('section') in [str(s) for s in global_preferences_registry.sections()]: # Check for global preferences return preference_form_builder(GlobalPreferenceForm, preferences, **kwargs) diff --git a/tabbycat/results/result.py b/tabbycat/results/result.py index a291f49ad96..57ce5ccd083 100644 --- a/tabbycat/results/result.py +++ b/tabbycat/results/result.py @@ -664,11 +664,11 @@ class DebateResultWithScoresMixin: uses_declared_winners = False uses_speakers = True - def __init__(self, ballotsub, load=True, criteria=[], **kwargs): + def __init__(self, ballotsub, load=True, criteria=None, **kwargs): super().__init__(ballotsub, load=False, **kwargs) self.positions = self.tournament.positions - self.criteria = criteria or [] + self.criteria = [] if criteria is None else criteria if load: if self.ballotsub.id is None: diff --git a/tabbycat/tournaments/models.py b/tabbycat/tournaments/models.py index 7fbfadf424c..789a56af96a 100644 --- a/tabbycat/tournaments/models.py +++ b/tabbycat/tournaments/models.py @@ -441,7 +441,7 @@ def num_available_adjudicators_not_allocated(self): # Draw retrieval methods # -------------------------------------------------------------------------- - def debate_set_with_prefetches(self, filter_args=[], filter_kwargs={}, ordering=(F('venue__name').asc(nulls_last=True),), + def debate_set_with_prefetches(self, filter_args=None, filter_kwargs=None, ordering=(F('venue__name').asc(nulls_last=True),), teams=True, adjudicators=True, speakers=True, wins=False, results=False, venues=True, institutions=False, check_ins=False, iron=False): """Returns the debate set, with aff_team and neg_team populated. @@ -451,6 +451,10 @@ def debate_set_with_prefetches(self, filter_args=[], filter_kwargs={}, ordering= from draw.models import DebateTeam from participants.models import Speaker from results.prefetch import populate_confirmed_ballots, populate_wins, populate_checkins + if filter_args is None: + filter_args = [] + if filter_kwargs is None: + filter_kwargs = {} debates = self.debate_set.filter(*filter_args, **filter_kwargs) if results: diff --git a/tabbycat/utils/admin.py b/tabbycat/utils/admin.py index deb91899425..fc61af715f9 100644 --- a/tabbycat/utils/admin.py +++ b/tabbycat/utils/admin.py @@ -27,8 +27,12 @@ def log_addition(self, request, object, message): def log_change(self, request, object, message): return super().log_change(request, object, self.add_ip_to_message(request, message)) - def log_deletion(self, request, object, object_repr, message=[]): + def log_deletion(self, request, object, object_repr, message=None): from django.contrib.admin.models import DELETION, LogEntry + + if message is None: + message = [] + return LogEntry.objects.log_action( user_id=request.user.pk, content_type_id=get_content_type_for_model(object).pk, diff --git a/tabbycat/utils/tables.py b/tabbycat/utils/tables.py index 31e09a090e6..a7c20110588 100644 --- a/tabbycat/utils/tables.py +++ b/tabbycat/utils/tables.py @@ -803,12 +803,14 @@ def add_ranking_columns(self, standings): } for ranking in standing.iterrankings()]) self.add_columns(headers, data) - def add_metric_columns(self, standings, integer_score_columns=[]): + def add_metric_columns(self, standings, integer_score_columns=None): """`integer_score_columns`, if given, indicates which metrics to cast to an int if the metric's value is an integer. For example, if the tournament preferences are such that the total speaker score should always be an integer, a list containing the string 'total' or 'speaks_sum' should be passed in via this argument.""" + if integer_score_columns is None: + integer_score_columns = [] headers = self._standings_headers(standings.metrics_info()) data = []