Conversation
not completley implemented yet.
not completley implemented yet.
modified: py3votecore/borda_at_large.py
modified: test_functionality/test_borda_at_large.py
modified: py3votecore/borda_manipulation_heurestics_methods.py modified: test_functionality/test_borda_manipulation_heurestics_methods.py
modified: py3votecore/borda_at_large.py modified: py3votecore/borda_manipulation_heurestics_methods.py modified: test_functionality/test_borda_manipulation_heurestics_methods.py
…exist modified: py3votecore/borda_manipulation_heurestics_methods.py modified: test_functionality/test_borda_manipulation_heurestics_methods.py
modified: py3votecore/borda_manipulation_heurestics_methods.py
modified: py3votecore/borda_manipulation_heurestics_methods.py
Working branch
modified: py3votecore/borda_manipulation_heurestics_methods.py modified: test_functionality/test_borda_manipulation_heurestics_methods.py
modified: test_functionality/test_borda_manipulation_heurestics_methods.py
modified: test_functionality/test_borda_manipulation_heurestics_methods.py
Working branch
new file: requirement.txt modified: setup.py modified: test_functionality/test_schulze_method.py
Added requirements and fixed a test.
modified: README.rst modified: py3votecore/borda.py modified: py3votecore/borda_at_large.py modified: py3votecore/borda_manipulation_heurestics_methods.py modified: setup.py modified: test_functionality/test_borda.py modified: test_functionality/test_borda_at_large.py modified: test_functionality/test_borda_manipulation_heurestics_methods.py
Added description of the added Borda votting rule and version update
| import copy | ||
|
|
||
| class BordaAtLarge(MultipleWinnerVotingSystem): | ||
|
|
| from .borda_at_large import BordaAtLarge | ||
|
|
||
| class Borda(AbstractSingleWinnerVotingSystem): | ||
|
|
modified: py3votecore/borda.py modified: py3votecore/borda_at_large.py
the-maldridge
left a comment
There was a problem hiding this comment.
Many small nits. This is a first pass review that does not check actual conformance against the algorithm in the linked whitepaper.
Each review item is commented on the first time I saw it, but multiple occurrences may exist within your patch. You will need to rebase your commits into a single logical change and push back to this PR prior to merge, but as I have not yet verified the correctness of the code, this is not required yet.
I found this PR difficult to review due to the lack of whitespace in most files. I strongly encourage you to refactor your code such that comments appear above the line they explain and to generally use whitespace to break up large blocks of code based on what each subsection does.
|
|
||
| def __init__(self, ballots, tie_breaker=None): | ||
| """ | ||
| The constructer accepts ballots of voters and a tie breaker if given. |
There was a problem hiding this comment.
Indeed it does expect ballots and a tie breaker, but the format would be useful to know for anyone consuming this class. You should amend the comment to include a brief description of the expected format of the ballots.
| @@ -0,0 +1,36 @@ | |||
| # Copyright (C) 2023, Leora Schmerler | |||
There was a problem hiding this comment.
name should be added to CONTRIBUTORS at the root of the repo.
|
|
||
| def calculate_results(self): | ||
| """ | ||
| calculate_results accepts an instance of Borda, and is called from the constructor. |
There was a problem hiding this comment.
Typically you would not state that it accepts anything if the only argument is the class instance itself. This should be reworded to describe what the function actually does.
|
|
||
| def as_dict(self): | ||
| """ | ||
| as_dict accepts an instance of Borda, and returns a dict of the BordaAtLarge. |
There was a problem hiding this comment.
Same as above, reword docstring.
| candidate will be elected by the Borda voting rule, tie-breaks in faver of the preferred_candidate. The algorithm outputs true if it succeeds | ||
| to find such manipulation, and false otherwise. | ||
|
|
||
| Programmer: Leora Schmerler |
There was a problem hiding this comment.
Move to CONTRIBUTORS in root.
|
|
||
| def AverageFit(ballots: list, preferred_candidate: str, k: int)->list or bool: | ||
| """ | ||
| "Complexity of and Algorithms for Borda Manipulation", by Jessica Davies, George Katsirelos, Nina Narodytska and Toby Walsh(2011), |
There was a problem hiding this comment.
If this reference is here to explain something you're going to need more words to explain that.
|
|
||
| setup(name='python3-vote-core', | ||
| version='20170329.00', | ||
| version='20230116.00', |
There was a problem hiding this comment.
This should be removed, as your PR is not what will trigger a release build.
| @@ -169,7 +169,10 @@ def test_tuple_ballots(self): | |||
| output_list = SchulzeMethod(input_list, ballot_notation=SchulzeMethod.BALLOT_NOTATION_GROUPING).as_dict() | |||
|
|
|||
| # Run tests | |||
There was a problem hiding this comment.
This appears to be unrelated to the rest of your PR, and should be part of a separate patch.
I added the Borda voting rule for one winner and for more winners, similar to the implementation of the Plurality voting rule. In addition, I added an implementation of the coalitional manipulation heuristics: Average Fit and Largest Fit.
Another minor fix in test_schulze_method: the test test_tuple_ballots somtimes fails, and somtimes does not, for the random tie-breaking order.