Remove ballots with candidate(s) cleaning function and update CleanedRankProfile index tracking

[graceg571]

Closes #381

Summary

remove_ballots_with_cand_rank_profile is added as a cleaning function to remove ballots containing a certain candidate or one from a set of candidates. Currently, specific candidate(s) can be removed from a ballot, but this supports workflows where ballots with an invalid write-in marker like "overvote" or "undervote" can be removed entirely.

Adding it surfaced that CleanedRankProfile's index sets can place ballots in the wrong buckets.

New paradigm for index tracking

Before: _iterate_and_clean_ranking_tuples produced a first-pass classification, and then individual cleaners "corrected" it. condense_rank_profile and remove_and_condense_rank_profile each ran a equivalence check
(_is_equiv_to_condensed, _is_equiv_for_remove_and_condense) over nonempty_altr_idxs and moved matching indices back into unaltr_idxs. This equivalence check tried to move ballots that were meaningfully the same before and after cleaning into unaltr_idxs but they were altered even if the candidates rankings positions were maintained.

Now: Now all four sets are derived in one place, from a direct comparison of each ballot's ranking tuple before and after cleaning:

set	definition
unaltr_idxs	orig_row == cleaned_row, identical before and after
no_rank_altr_idxs	cleaned ranking is entirely frozenset() and/or frozenset("~"), minus unaltr_idxs
nonempty_altr_idxs	everything else is altered, and still holds at least one candidate set
no_wt_altr_idxs	always empty, ranking cleaning currently never changes a weight

Both equivalence check functions were deleted.

Things to note about this change:

• unaltr_idxs includes dropped ballots, because dropping is not altering. A ballot removed for being null or zero weight was already in unaltr_idxs because they are not touched by the cleaning function. To find what was dropped, compare the cleaned profile's index against the parent's index.
• remove_ballots_with_cand_rank_profile does not call clean_rank_profile because it acts more as a filter, then cleaner. It alters no ranking, and so reports every parent index as unaltered.

Changes

• remove_ballots_with_cand_rank_profile is added to rank_profiles_cleaning.py. Ballots are matched using the candidates' integer IDs in the internal df. Removed ballots are treated the same as dropped null-ranking and zero-weight ballots, i.e. gaps in the index relative to the parent profile.
• no_rank_altr_idxs now includes ballots where every rank slot is frozenset() or frozenset("~"). This covers ballots where the cleaning function removed all ranked candidates and left empty frozensets behind.
• unaltr_idxs is subtracted from no_rank_altr_idxs, so ballots that were already empty are distinguished from ballots that became empty through cleaning.
• unaltr_idxs now also covers ballots that were dropped but never modified. This already included dropped null-ranking and zero-weight ballots, and now includes the ballots removed by remove_ballots_with_cand_rank_profile.
• _is_equiv_to_condensed and _is_equiv_for_remove_and_condense are removed.
• _validate_candidate_names takes a context argument, either a SourceWithAttribute dataclass or a plain variable name instead of separate source/attribute parameters. So, callers can get an informative error message about the source of their candidates. This cascades through ballot.py,pref_profile.py, and the bloc-slate config modules.
• Docstrings corrections

Testing

• test_remove_ballots_with_cand_rank_profile.py added, covering removed candidates with and without ties, chaining with remove_cand, and the index-set bookkeeping.
• test_clean_ranked_profile.py and test_remove_cand_ranked_profile.py updated for the no_rank_altr_idxs / unaltr_idxs changes.
• test_condense_ranked_profile.py updated for the removal of the condense equivalence functions.
• test_collections.py, test_bloc_slate_config.py, test_RankBallot.py, and test_common_utils.py updated for the _validate_candidate_names signature change.

[peterrrock2]

Looking good so far!

[peterrrock2]

Fixtures
Test all attrs for CleanProfile independently
Test chaining
Test idempotent

[graceg571]

Added idempotent test that confirmed the result is the same after another application of the function. Added chaining test pin that remove ballots with a candidate and remove candidates from a ballot do not modify each other's results. All the attributes are tested for CleanProfile. Can split into a separate test for each attribute if desired.

[peterrrock2]

Great! We should also make sure to check steps in the chain robustly here. So doing more of

def test_remove_ballots_with_different_candidates_chaining(profile_no_ties):
    first = remove_ballots_with_cand_rank_profile("A", profile_no_ties)

    assert first.parent_profile is profile_no_ties
    assert list(first.df.index) == first.df_index_column == [1, 2]
    assert first.unaltr_idxs == {0, 1, 2, 3}

    second = remove_ballots_with_cand_rank_profile("B", first)

    assert second.parent_profile is first
    assert list(second.df.index) == second.df_index_column == [2]
    assert second.unaltr_idxs == {1, 2}

    for cleaned in (first, second):
        assert cleaned.no_rank_altr_idxs == set()
        assert cleaned.no_wt_altr_idxs == set()
        assert cleaned.nonempty_altr_idxs == set()

which checks not only the profille after two rounds of cleaning, but also checks the parent and that the unalter_indxs of the child refer to the immediate parent (this is very much in the vein of dotting t's and crossing i's).

The idempotence tests are good (removing "A" twice), but we should aim to cover all cases.

[peterrrock2]

Document change in PR description, and update the doc string in the CleanProfile class. Please add examples in that doc string so we can more easily determine the meaning of these.

TODO:

Add issue where we use a dataclass to descriptively partition the possible state space for these alterations -- users should make the decision on what they car about

example

@dataclass(slots = true)
class CleaningIndexDeltas:
    dropped: set or list or tuple --> entire row removed index does not exist in child
    remove_empty_ranking: TypedDict { trailing_only: <object>, internal_only: <object>, both_trailing_and_internal: <object>}
    subset_previous_ballot: TypedDict {strict_subset_no_gaps ([A, B, {C, D}, E] -> [B, {C,D}]), strict_subset_gaps, weak_subset ([A, B, {C,D}, E] -> [B, C]) 
... and so on

This is a loose sketch

[graceg571]

I added an example to the CleanedRankProfile class and updated the docstring to reflect the current paradigm. Will add an issue.

[graceg571]

Issue here: #392

[peterrrock2]

The current paradigm (which is not great) records dropped ballots as gaps in the index (compared to the parent profile)

[graceg571]

Updated the docstring to reflect dropped ballots as gaps in the index. I pointed to that dropped empty ranking and zero weight ballots are recorded in the same manner.

[peterrrock2]

Suggested change

	raise ProfileError("Profile must be a RankProfile.")
	raise TypeError("Profile must be a RankProfile.")

[graceg571]

Replaced all instances of ProfileError with TypeError within rank_profiles_cleaning.py. 1 test was broken and fixed with TypeError.

[peterrrock2]

I think that we have a function that checks if something is a valid candidate that we should use here

[graceg571]

Replaced with _validate_candidate_names

[peterrrock2]

Suggested change

removed_ballot_idxs = set(profile.df[ballots_to_remove].index)

[graceg571]

The removed ballots are not considered no_rank_altr_idxs but are just dropped, no alteration of their ranking nor weight. Their removal is accounted for in the difference between the parent profile's index and the cleaned profile's index. This applies to removed empty ranking and zero weight ballots, too.

[peterrrock2]

Suggested change

	respect to ``parent_profile.df``. A ballot has no ranking after cleaning if its ranking
	contains only empty or tilde sets, and it had at least one valid candidated ranked
	before cleaning.
	respect to ``parent_profile.df``. A ballot index can only be a member for the `no_rank_altr_idxs`
	set if, after cleaning, the ballot consists of only elements of the form `frozenset()` (the
	empty frozen set) or `frozenset({'~'})` (the set of the special character "tilde"), and it is not
	identical to itself before applying the cleaning operation.

[graceg571]

Updated to follow this rule: "If a ballot, as a result of cleaning, does not belong to unaltr_idxs and contains at least one ranking position with frozenset(<SET_OF_CANDS>) where <SET_OF_CANDS> is a non-empty set of valid candidate identifiers, then that ballot goes in nonempy_altr_idxs."

[peterrrock2]

We should use some more descriptive types her to avoid this:

   source_description = f"{source.__class__.__name__}.{attribute}" if source else f"{attribute}"

c.f. our Slack messages

[graceg571]

Added a dataclass for a object with attribute and TypeAlias for variable names to describe what we expect for the context of error messages when validating candidate's name.

[peterrrock2]

Nit:

Suggested change

	idx for idx, c in zip(idxs, cleaned_rows) if all(x == tilde or x == empty for x in c)
	idx for idx, cln_row in zip(idxs, cleaned_rows) if all(cand_set == tilde or cand_set == empty for cand_set in cln_row)

[peterrrock2]

We should leave this as True

[graceg571]

Agreed. Reverted. Will add an issue around visiting the default values for these cleaning flags.

[graceg571]

Issue here: #396

[peterrrock2]

Great! Thanks!

[peterrrock2]

How to partition the ballots after cleaning:

• If a ballot is identical before and after cleaning, it goes in unaltr_idxs.
• If a ballot, as a result of cleaning, goes from having positive weight to no weight, then it goes in no_wt_altr_idxs.
• If a ballot, as a result of cleaning, contains only frozenset() and frozenset({'~'}), and is not a member of unaltr_idxs, then it goes in no_rank_altr_idxs
• If a ballot, as a result of cleaning, does not belong to unaltr_idxs and contains at least one ranking position with frozenset(<SET_OF_CANDS>) where <SET_OF_CANDS> is a non-empty set of valid candidate identifiers, then that ballot goes in nonempy_altr_idxs.

[peterrrock2]

Make sure to add unit tests for exactly these cases if they are attainable for each cleaning function. In particular, for each defined cleaning function, try to suss out all of the edge cases for the last 2 bullets.

[graceg571]

Updated the docstring for CleanedRankProfile within cleaned_pref_profile.py to capture this partition.

This means that _is_equiv_to_condensed and _is_equiv_to_remove_and_condensed are unnecessary functions to add back additional_unaltr_idxs that are not identical rankings before and after cleaning but their information is not meaningfully changed. For example, a ballot that is ["A", "B", {}] where rank position 3 is empty would be ["A", "B"] after condensing. Candidates have not changed their ranking but rank position 3's empty set has been replaced with ``frozenset({'~'})(special character tilde). Prior,_is_equiv_to_condensed` would add this ballot's index to `unaltr_idxs` but with the updated strict paradigm, this ballot has been altered and should be a member of the `nonempty_altr_idxs` set.

Updated cleaning functions tests to test edge cases for nonempty_altr_idxs and no_rank_altr_idxs.

[peterrrock2]

Nearly done! A couple of small things, and we should be good to go

[peterrrock2]

Bug: the post-init will never run

Suggested change

	def __post_init(self):
	def __post_init(self)__:

[graceg571]

Good catch! Thanks! Confirmed it runs now.

[peterrrock2]

We need to update the doc strings for condense_rank_profile and remove_and_condense_rank_profile: they both still say that condensing trailing empty positions leaves a ballot "unaltered" and we changed things so that replacing trailing empty sets with padding now puts the index in nonempty_altr_idxs

[graceg571]

Updated doc strings for condense_rank_profile and remove_and_condense_rank_profile to differentiate trailing empty sets that go in nonempty_altr_idxs and only empty sets or a mix with tilde sets that goes in no_rank_altr_idxs. I explained trailing empty sets are replaced with frozenset({'~'}) while empty sets between ranking positions with candidate sets are removed and the rest of the ranking is shifted up.

[peterrrock2]

I am realizing that we should use getattr_static here since we might have some uninitialized values. The getattr_static function checks for declared slots, which is all we need here.

[graceg571]

getattr_static would cover the case where a source is a __slots__ class object and its attribute has not been initialized but is a declared slot and attribute of the object. I updated __post_init__ to use inspect.getattr_static after checking if hasattr is True. SlateCandMap stores its parent (BlocSlateConfig) as a proxy and its attributes are accessible through hasattr not getattr_static.

[peterrrock2]

Great! We should also make sure to check steps in the chain robustly here. So doing more of

def test_remove_ballots_with_different_candidates_chaining(profile_no_ties):
    first = remove_ballots_with_cand_rank_profile("A", profile_no_ties)

    assert first.parent_profile is profile_no_ties
    assert list(first.df.index) == first.df_index_column == [1, 2]
    assert first.unaltr_idxs == {0, 1, 2, 3}

    second = remove_ballots_with_cand_rank_profile("B", first)

    assert second.parent_profile is first
    assert list(second.df.index) == second.df_index_column == [2]
    assert second.unaltr_idxs == {1, 2}

    for cleaned in (first, second):
        assert cleaned.no_rank_altr_idxs == set()
        assert cleaned.no_wt_altr_idxs == set()
        assert cleaned.nonempty_altr_idxs == set()

which checks not only the profille after two rounds of cleaning, but also checks the parent and that the unalter_indxs of the child refer to the immediate parent (this is very much in the vein of dotting t's and crossing i's).

The idempotence tests are good (removing "A" twice), but we should aim to cover all cases.

[peterrrock2]

I think that there are just a couple of doc string updates for this and then we can call this good. Great work!

[peterrrock2]

Propagate the doc string update for remove_null_ballot

[graceg571]

remove_null_ballot arg all have the same doc string now. Updated the condense cleaning functions docstrings to be a bulleted list along with some spelling fixes.