Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/votekit/cleaning/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
remove_and_condense_rank_profile,
remove_cand_rank_profile,
remove_repeat_cands_rank_profile,
truncate_rank_profile,
)
from .score_ballots_cleaning import remove_cand_score_ballot
from .score_profiles_cleaning import clean_score_profile, remove_cand_score_profile
Expand All @@ -22,6 +23,7 @@
"remove_cand_rank_ballot",
"condense_rank_ballot",
"remove_repeat_cands_rank_ballot",
"truncate_rank_profile",
"remove_cand_score_ballot",
"clean_score_profile",
"remove_cand_score_profile",
Expand Down
77 changes: 77 additions & 0 deletions src/votekit/cleaning/rank_profiles_cleaning.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,83 @@ def remove_cand_rank_profile(
)


def truncate_ranking_row(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would update name to be something like truncate_at_cand_ranking_row or truncate_ranking_row_at_cand to be more specific about the type of truncation. Same comment for truncate_rank_profile.

removed: Candidate | CandidateList,
ranking_tup: tuple[frozenset, ...],
) -> tuple[frozenset, ...]:
"""
Truncate a ranking at the first position containing a specified candidate or marker.

The matching position and every position below it are replaced with trailing ``~``
placeholders so that the ranking keeps its original width in a profile dataframe.

Args:
removed (Candidate | list[Candidate]): Candidate or list of candidates or markers at
which to truncate.
ranking_tup (tuple): Ranking to truncate.

Returns:
tuple: Ranking truncated at the first matching position.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update docstring to tuple[frozenset, ...] to match the function signature. It's better to be specific because a user could read the docstring and assume they can pass any tuple as an argument. I see this is done throughout the file so I will update the doc strings elsewhere!

"""
if isinstance(removed, Candidate):
removed = [removed]

removed_set = set(removed)
out: list[frozenset] = []

for cand_set in ranking_tup:
if cand_set.isdisjoint(removed_set):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, this will truncate ballots where a candidate within the removed_set exists. So, it will truncate where that candidate may be tied with other non-removed candidates. This is covered in your documentation but could be nice to cover in a test.

out.append(cand_set)
continue

out.extend([frozenset("~")] * (len(ranking_tup) - len(out)))
break
Comment on lines +315 to +321

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Suggest to make an explicit if/else. The if and trailing break reads as two statements to trace when it's really one decision: add the cand_set or truncate at removed_set. Or, have only one if that checks whether the cand_set contains a removed_set candidate, truncate and break if so and add the cand_set to the cleaned ranking as the default.


return tuple(out)


def truncate_rank_profile(
removed: Candidate | CandidateList,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would move away from removed because this function is not only removing this candidate but removing all rankings/candidates below that candidate.

profile: RankProfile,
remove_empty_ballots: bool = True,
remove_zero_weight_ballots: bool = True,
retain_original_candidate_list: bool = True,
) -> CleanedRankProfile:
"""
Truncate ranked ballots at the first position containing a specified candidate or marker.

This is useful for cleaning CVR data where values such as ``"overvote"`` or ``"undervote"``
terminate the meaningful portion of a ballot. The matching position and all lower-ranked
positions are removed. Ballots without a matching value are retained unchanged.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right that this function is especially useful for cleaning CVR data with ballot error markers or end-of-ballot markers but it doesn't need to be captured in the docstring. And then add "Wrapper for clean_rank_profile that does some extra processing to ensure the candidate list is handled correctly."


Args:
removed (Candidate | list[Candidate]): Candidate, marker, or list of candidates and
markers at which to truncate.
profile (RankProfile): Profile to truncate.
remove_empty_ballots (bool, optional): Whether or not to remove ballots with no ranking
after truncation. Defaults to True.
remove_zero_weight_ballots (bool, optional): Whether or not to remove zero-weight ballots.
Defaults to True.
retain_original_candidate_list (bool, optional): Whether or not to retain the original
candidate list. Defaults to True.

Returns:
CleanedRankProfile: A cleaned ``RankProfile``.

Raises:
ProfileError: Profile must only contain ranked ballots.
"""
cleaned_profile = clean_rank_profile(
profile,
partial(truncate_ranking_row, removed),
remove_empty_ballots,
remove_zero_weight_ballots,
retain_original_candidate_list,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add candidate list handling here for retain_original_candidate_list. The candidate list is important metadata to maintain for an election/profile. User expects the "truncate_at" candidate to be removed but may want to retain all other candidates even if after truncation, some are no longer casted. Follows the implementation of the remove candidate cleaning functions.

return cleaned_profile


def condense_ranking_row(
ranking_tup: tuple,
) -> tuple:
Expand Down
52 changes: 52 additions & 0 deletions tests/cleaning/rank_profiles/test_truncate_ranked_profile.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import pytest

from votekit.ballot import RankBallot, ScoreBallot
from votekit.cleaning import truncate_rank_profile
from votekit.pref_profile import CleanedRankProfile, ProfileError, RankProfile, ScoreProfile

profile = RankProfile(
ballots=[
RankBallot(ranking=[{"A"}, {"overvote"}, {"B"}, {"C"}], weight=1),
RankBallot(ranking=[{"A"}, {"B"}, {"C"}], weight=2),
RankBallot(ranking=[{"undervote"}, {"C"}], weight=3),
RankBallot(ranking=[{"A"}, {"B"}], weight=0),
]
)


def test_truncate_rank_profile_at_candidate_or_marker():
cleaned_profile = truncate_rank_profile(["overvote", "undervote"], profile)

assert isinstance(cleaned_profile, CleanedRankProfile)
assert cleaned_profile.parent_profile == profile
assert cleaned_profile.ballots == (
RankBallot(ranking=[{"A"}], weight=1),
RankBallot(ranking=[{"A"}, {"B"}, {"C"}], weight=2),
)
assert cleaned_profile.no_rank_altr_idxs == {2}
assert cleaned_profile.nonempty_altr_idxs == {0}
assert cleaned_profile.unaltr_idxs == {1, 3}
assert cleaned_profile.no_wt_altr_idxs == set()


def test_truncate_rank_profile_can_retain_empty_and_zero_weight_ballots():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test does not cover retaining empty ballots. Only zero weight ballots.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The undervote ballot would become an empty ballot after cleaning. Could truncate at that candidate to test empty ballots are retained.

cleaned_profile = truncate_rank_profile(
"overvote",
profile,
remove_empty_ballots=False,
remove_zero_weight_ballots=False,
)

assert cleaned_profile.ballots == (
RankBallot(ranking=[{"A"}], weight=1),
RankBallot(ranking=[{"A"}, {"B"}, {"C"}], weight=2),
RankBallot(ranking=[{"undervote"}, {"C"}], weight=3),
RankBallot(ranking=[{"A"}, {"B"}], weight=0),
)


def test_truncate_rank_profile_requires_rank_profile():
score_profile = ScoreProfile(ballots=[ScoreBallot(scores={"A": 1})])

with pytest.raises(ProfileError, match="Profile must be a RankProfile."):
truncate_rank_profile("overvote", score_profile) # type: ignore[arg-type]