Add ranked ballot marker filter - #382
Conversation
fetachino
left a comment
There was a problem hiding this comment.
I traced the filtering path through the Ranking_* frozensets, including markers inside tied ranks and multiple removal markers. Using isdisjoint here correctly drops the whole ballot as soon as any specified marker appears, while preserving the original candidate list and parent/index metadata for retained rows. The public export and the remove_zero_weight_ballots=False case are also covered.
I ran the new test together with the neighboring ranked-profile cleaning tests: 11 passed. Ruff and ty check also pass for the touched cleaning code and test. I did not find a blocking correctness or maintainability issue.
|
Hey @0xTaoZ! Thank you so much for contributing to VoteKit! It's super exciting to see our work being used by people outside of our community. If you happen to have a moment, I would love to hear more about how you found the package and what you are using it for so that we can better fit the needs of our users. You may have noticed that we made our latest release for VoteKit around two weeks ago. Since that time, I have been preoccupied with some other projects and did not notice the latest set of PRs that came in. Unfortunately, this led to me asking another member of the lab to work on this exact issue. They implemented a slightly faster version than the one that you have presented here that leverages the That being said, I would still be thrilled to have your contributions on this project. If you would like, issue #322 is similar to this one and, if you comment on the thread in that issue, I or one of the other maintainers of the project would be happy to assign you that issue so no one else takes it. I'll leave this PR open until Wednesday, August 26th so that you have an opportunity to respond. Additional notes (this section is a form that I am including on several recent PRs) Since there have been so many new contributors in the last couple of weeks (a wonderful problem to have), I thought it important to update the "Use of AI Tools" section of the Contributing.md on our main branch to clarify what we currently consider to be best-practices for our lab. To be clear, this this is not an indictment of the use of any AI tools or a statement about the work presented here (indeed, it would be a bit illogical to find anyone at issue with this policy before it was explicitly stated!). I include it here only because this is 1. a recent update in a document people infrequently visit and 2. an effort to clearly communicate our expectations around contributing to this project. I'll include the exact snippet here so you don't have to go through the trouble of finding it yourself:
Thank you again for your contribution, and I hope that we'll see more from you! -Peter |
Summary
remove_rank_ballots_with_candsfor dropping ranked ballots containing specified candidates or CVR markersvotekit.cleaningFixes #381.
Testing
uv run pytest tests/cleaning/rank_profiles/test_remove_rank_ballots_with_cands.py tests/cleaning/rank_profiles/test_remove_cand_ranked_profile.py tests/cleaning/rank_profiles/test_remove_and_condense_ranked_profile.py -quv run ruff format --check src/votekit/cleaning tests/cleaning/rank_profiles/test_remove_rank_ballots_with_cands.pyuv run ruff check src/votekit/cleaning tests/cleaning/rank_profiles/test_remove_rank_ballots_with_cands.pyuv run ty check src/votekit/cleaning tests/cleaning/rank_profiles/test_remove_rank_ballots_with_cands.pyuv run pytest tests/cleaning -qgit diff --checkI also tried broader repository checks.
uv run pytest tests -qstops before this change on optional/environment-dependent tests:tests/test_animations.pyneedsmanim, and URL-loading tests fail local TLS verification against GitHub.uv run ruff format --check src tests && uv run ruff check src testspasses. Repo-wideuv run ty check src testsstops on the same missing optional animation modules (manim,manimpango).