Feat/sum mult profiles by graceg571 · Pull Request #369 · mggg/VoteKit

graceg571 · 2026-06-01T13:55:30Z

Closes #331

Adds a utility method 'sum_profiles' for summing a list of profiles (RankProfile or ScoreProfile).

Implementation

Iterates through the list and accumulates with +, delegating the actual combination to the existing __add__ methods on RankProfile and ScoreProfile.

Raises ValueError on empty list because the correct empty profile type can't be inferred. Raises TypeError if a mix of RankProfiles and ScoreProfiles are given as input. Checks before iterating through list to avoid performing unnecessary adds.

Tests

Added test cases for:

sum_profiles([...]) with an empty list
sum_profiles([...]) with a list of 1 profile
sum_profiles([...]) with a list of mixed ScoreProfiles and RankProfiles
sum_profiles([...]) for list of ScoreProfiles and list of RankProfiles

EdouardHeitzmann · 2026-06-03T05:11:02Z

Just a thought, but using a python loop to repeatedly call RankProfile.__add__(...) can be computationally expensive below:

for profile in profiles[1:]:
        combined_profiles = combined_profiles + profile

It could speed things up to copy all the underlying profile.dfs into a list, then call pd.concat once on the whole list once -- it's a quadratic speedup in the number of profiles copied (I think!), since for a list of n profiles the first once gets copied n-1 times, the next one n-2, etc under the current setup.

EdouardHeitzmann · 2026-06-03T05:46:33Z

Upon further thought, mimicking the RankProfile.__add__ logic for the many-profiles version of addition as I suggest above would introduce some maintainability concerns (since the same logic would then exist in two separate places). Maybe a clean fix would be to move out the logic currently in __add__ into the sum_profiles helper you're currently working on, make it compatible with arbitrary-length lists of profiles, and then call sum_profiles inside of __add__:

def __add__(self, other) -> RankProfile:
        """
        Add two PreferenceProfiles by combining their ballot lists.
        """
        if not isinstance(other, RankProfile):
            raise TypeError("Unsupported operand type. Must be an instance of RankProfile.")

        # insert any other validation here

        return sum_profiles([self, other])

Sorry to be nosing in to this PR, I think the work you're doing is awesome!

peterrrock2

Notes:

Move to internal methods with guaranteed return types (e.g. _sum_rank_profiles)

peterrrock2 · 2026-06-03T20:14:29Z

+def sum_profiles(profiles: Sequence[RankProfile | ScoreProfile]) -> RankProfile | ScoreProfile:
+    """
+    Combines multiple PreferenceProfiles by combining their ball lists.
+
+    Args:
+        profiles (Sequence[PreferenceProfile]): The profiles to sum.
+
+    Returns:
+        PreferenceProfile: The combined preference profile.
+
+    Raises:
+        ValueError: Cannot sum an empty list of profiles.
+        TypeError: All profiles must be of the same type.
+    """
+
+    from votekit.pref_profile.pref_profile import RankProfile, ScoreProfile
+
+    if len(profiles) == 0:
+        raise ValueError("Cannot sum an empty list of profiles.")
+
+    if len(profiles) == 1:
+        return profiles[0]


Suggested change

def sum_profiles(profiles: Sequence[RankProfile | ScoreProfile]) -> RankProfile | ScoreProfile:

"""

Combines multiple PreferenceProfiles by combining their ball lists.

Args:

profiles (Sequence[PreferenceProfile]): The profiles to sum.

Returns:

PreferenceProfile: The combined preference profile.

Raises:

ValueError: Cannot sum an empty list of profiles.

TypeError: All profiles must be of the same type.

"""

from votekit.pref_profile.pref_profile import RankProfile, ScoreProfile

if len(profiles) == 0:

raise ValueError("Cannot sum an empty list of profiles.")

if len(profiles) == 1:

return profiles[0]

def sum_profiles(profiles: Sequence[RankProfile | ScoreProfile]) -> RankProfile | ScoreProfile:

"""

Combines multiple PreferenceProfiles by combining their ball lists.

Args:

profiles (Sequence[PreferenceProfile]): The profiles to sum.

Returns:

PreferenceProfile: A new PreferenceProfile object containing the combined preference profile.

Raises:

ValueError: Cannot sum an empty list of profiles.

TypeError: All profiles must be of the same type.

"""

from votekit.pref_profile.pref_profile import RankProfile, ScoreProfile

if len(profiles) == 0:

raise ValueError("Cannot sum an empty list of profiles.")

if len(profiles) == 1:

return profiles[0] # Change to copy. Might be worth adding a __copy__ method

Added a copy method to RankProfile and ScoreProfile. Updated the return comment

peterrrock2 · 2026-06-10T20:18:55Z

Upon further thought, mimicking the RankProfile.__add__ logic for the many-profiles version of addition as I suggest above would introduce some maintainability concerns (since the same logic would then exist in two separate places). Maybe a clean fix would be to move out the logic currently in __add__ into the sum_profiles helper you're currently working on, make it compatible with arbitrary-length lists of profiles, and then call sum_profiles inside of __add__:
def __add__(self, other) -> RankProfile:
        """
        Add two PreferenceProfiles by combining their ballot lists.
        """
        if not isinstance(other, RankProfile):
            raise TypeError("Unsupported operand type. Must be an instance of RankProfile.")

        # insert any other validation here

        return sum_profiles([self, other])
Sorry to be nosing in to this PR, I think the work you're doing is awesome!

No worries @EdouardHeitzmann. Thank you for including your input!

peterrrock2

Just a couple of quick things on this!

peterrrock2 · 2026-06-10T20:30:53Z

+    total_cand = set().union(*[set(profile.candidates) for profile in score_profiles])
+    total_dfs = []
+    for p in score_profiles:
+        curr_df = p.df.copy()
+        curr_cand = set(p.candidates)
+        for cand in total_cand - curr_cand:
+            curr_df[cand] = [np.nan] * len(curr_df)
+        total_dfs.append(curr_df)


We could speed this up a little bit by not copying the dataframes that already contain a score for every candidate in total_cand. Avoids a secondary df copy.

Did not copy the data frame if they already contain a score for each candidate or the same ranking length for summing ScoreProfiles and RankProfiles respectively.

peterrrock2 · 2026-06-10T20:35:00Z

+        max_ranking_length=3,
+    )
+    summed_profile = sum_profiles([profile])
+    assert summed_profile == profile


Suggested change

assert summed_profile == profile

assert summed_profile == profile

assert id(summed_profile) != id(profile) # make sure that we return a copy

peterrrock2 · 2026-06-10T20:35:50Z

+        candidates=["A", "B", "C", "D"],
+    )
+    summed_profile = sum_profiles([profile])
+    assert summed_profile == profile


Suggested change

assert summed_profile == profile

assert summed_profile == profile

assert id(summed_profile) != id(profile)

Grace Gibson added 2 commits May 29, 2026 15:28

summing profile fxn

57e7cc0

add sum profile tests

201988a

graceg571 requested a review from peterrrock2 June 1, 2026 13:55

graceg571 self-assigned this Jun 1, 2026

graceg571 changed the base branch from main to 3.4.1 June 1, 2026 20:33

move sum logic to sum_profiles instead of add

db1f394

peterrrock2 requested changes Jun 3, 2026

View reviewed changes

graceg571 added 2 commits June 5, 2026 09:46

fix typing, add copy method

e4be73f

fix return comment of sum_profiles

a33baba

peterrrock2 requested changes Jun 10, 2026

View reviewed changes

avoid unnecessary copy, confirm copy in test

bf7e180

	assert summed_profile == profile
	assert summed_profile == profile
	assert id(summed_profile) != id(profile) # make sure that we return a copy

Conversation

graceg571 commented Jun 1, 2026

Implementation

Tests

Uh oh!

EdouardHeitzmann commented Jun 3, 2026

Uh oh!

EdouardHeitzmann commented Jun 3, 2026

Uh oh!

peterrrock2 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peterrrock2 commented Jun 10, 2026

Uh oh!

peterrrock2 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants