Conversation
|
Hey @fetachino! Thank you so much for contributing to VoteKit! It's wonderful 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 of VoteKit around two weeks ago. Since then, I have been preoccupied with some other projects and missed the latest set of PRs that came in. This looks like a great start on this issue, and one of the other maintainers will be adding a review with some additional feedback shortly. The following section may or may not pertain to the work that you have done here, but, since we have had a small rush of PRs from new contributors in recent days, I thought it a good idea to include this message on all of the recent PRs. 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 for looking at #383!!), and I hope that we'll see more from you soon! -Peter |
graceg571
left a comment
graceg571
left a comment
There was a problem hiding this comment.
Looks good! Good call to use the original profile's candidates list regardless of retain_original_candidate_list's setting. Just 2 comments. 1 about adding an additional test and another about writing your own _is_equiv function.
| if tuple(ranking_df.loc[i]) | ||
| == remove_repeat_cands_from_ranking_row(tuple(ranking_df.loc[i])) | ||
| and _is_equiv_to_condensed(ranking_df.loc[i]) # type: ignore[arg-type] |
There was a problem hiding this comment.
Move to its own function that can call _is_equiv_to_condensed internally. Follows the pattern of other functions that condense ballots and clarifies what ballots are altered versus unaltered via your cleaning function.
| profile = RankProfile( | ||
| ballots=[ | ||
| RankBallot(ranking=[{"A"}, {"A"}, {"B"}, {"C"}]), | ||
| RankBallot(ranking=[{"A", "C"}, {"C"}, frozenset(), {"B"}]), |
There was a problem hiding this comment.
Add a case where the repeated candidate is in a tie. Not just the first instance of a candidate within a ranking.
|
@fetachino just following up to see if you saw the above messages. Thanks! |
|
Continuing the conversation from #383.
Like I said, I would love to hear more about the work that you are doing whenever you get a chance! Also a clarification @fetachino, does this mean that you intend to continue to work on this PR? It would seem so, and I would love to have your contribution, but I want to make sure. Let me know! |
3 participants
Summary
votekit.cleaningWhy
Callers currently need to chain duplicate removal and condensation, creating an intermediate cleaned profile. This adds the combined operation requested in #275, following the existing optimized remove-and-condense pattern.
Validation
py -m uv run pytest tests/cleaning/rank_profiles(19 passed)py -m uv run ruff check src/votekit/cleaning tests/cleaning/rank_profiles/test_remove_repeat_cands_and_condense_rank_profile.pypy -m uv run ruff format --check src/votekit/cleaning tests/cleaning/rank_profiles/test_remove_repeat_cands_and_condense_rank_profile.pypy -m uv run ty check src/votekit/cleaning/rank_profiles_cleaning.pyCloses #275.