Conversation
Bug fix: now considers unranked candidates as tied for last place, and thus able to be vetoed. Various optimizations, most importantly relying on profile.df instead of duplicated Ballot objects to conduct election. For deterministic tiebreak, approximately 50-60x faster than previous implementation (testing on a 3-candidate election with 26k voters; was ~2.8s, now <0.05s). For random tiebreak, only about 8-10x faster. Now supports `tiebreak` = 'random', 'first_place', 'borda', and 'lex'. `tiebreak` is no longer permitted to be `None`. The default value is 'first_place', and the backup tiebreak is always 'lex', i.e., lexicographic/alphabetical. Because PluralityVeto requires tiebreaking at the ballot level (rather than round level), PluralityVeto does not report tiebreaks to ElectionState. However, a new attribute `tiebreak_order` allows users to inspect the tiebreak order when using a non-random tiebreak method. Refactored significantly for readability. Corrected existing documentation and added thorough documentation and comments.
…elimination_strategy' param to choose between PluralityVeto and SerialVeto
…subclasses: PluralityVeto and SerialVeto, which differ only by their implementation of _veto_loop.
peterrrock2
left a comment
There was a problem hiding this comment.
This is looking great!
Small nit: I am trying to unify the doc string style accoss the repository. Can you modify the doc strings to look more like:
def foo( arg1: str | None, arg2: int = 3) -> str:
"""
Brief description (max 100 chars including indents)
More details (max 100 chars per line, but as many lines as you would like)
Example:
<Stuff goes here. This section is optional>
Args:
arg1 (str | None): description
arg2 (int, optional): description. Defaults to 3.
Returns:
str: The string "baz"
"""
...This is a mild modification of the Google-style doc string and adheres to the PEP 257 recommendation but with the line width modified to 100 chars rather than 80 (better for 16:9 screens IMO)
Not a small nit at all: the docstrings definitely needed some love! Check it out now. |
Overhauled
PluralityVeto.Bug fix: now considers unranked candidates as tied for last place, and thus able to be vetoed.
Various optimizations, most importantly relying on profile.df instead of duplicated Ballot objects to conduct election. For deterministic tiebreak, approximately 50-60x faster than previous implementation (testing on a 3-candidate election with 26k voters; was ~2.8s, now <0.05s). For random tiebreak, only about 8-10x faster.
Refactored significantly for readability. Corrected existing documentation and added thorough documentation and comments.
Updated tests for PluralityVeto.
m(number of seats) now defaults to 1.Now supports
tiebreak= 'random', 'first_place', 'borda', and 'lex'.tiebreakis no longer permitted to beNone. The default value is 'first_place', and the backup tiebreak is always 'lex', i.e., lexicographic/alphabetical.Because PluralityVeto requires tiebreaking at the ballot level (rather than round level), PluralityVeto does not report tiebreaks to ElectionState. However, a new attribute
tiebreak_orderallows users to inspect the tiebreak order when using a non-random tiebreak method.