Concise STV #6
Open
Concise STV #6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
user2589
commented
Mar 14, 2018
| @@ -21,50 +21,20 @@ class Candidate: | |||
| """ | |||
|
|
|||
| def __init__(self, uid, name=None): | |||
| """Initializes Candidate with name and uid. | |||
There was a problem hiding this comment.
standard methods don't really need docstrings
user2589
commented
Mar 14, 2018
| @@ -11,7 +11,7 @@ | |||
| __status__ = "Production" | |||
|
|
|||
|
|
|||
| class Candidate: | |||
| class Candidate(object): | |||
There was a problem hiding this comment.
old style classes might cause issues on some platforms
user2589
commented
Mar 14, 2018
| return 'VoteTracker(votes_for_candidate={!r}, votes_cast={!r})'.format( | ||
| self._votes_for_candidate, self.votes_cast) | ||
|
|
||
| def decription(self): | ||
| """Returns a printable long-form user representation of the VoteTracker. | ||
|
|
||
| Returns: | ||
| String containing the printable representation of the VoteTracker. | ||
| :return String containing the printable representation of the |
There was a problem hiding this comment.
There is a standard for autodocumentation:
https://thomas-cokelaer.info/tutorials/sphinx/docstring_python.html
user2589
commented
Mar 14, 2018
| @@ -863,7 +851,7 @@ def test_cgp_grey_animal_kingdom(self): | |||
| tiger = Candidate('tiger', name='Tiger') | |||
| lynx = Candidate('lynx', name='Lynx') | |||
|
|
|||
| expected_winners = set([gorilla, monkey, tiger]) | |||
| expected_winners = {gorilla, monkey, tiger} | |||
There was a problem hiding this comment.
set comprehensions:
https://www.smallsurething.com/list-dict-and-set-comprehensions-by-example/
user2589
commented
Mar 14, 2018
tests.py
Outdated
| candidates.append(candidate_for_id[candidate_id]) | ||
| return candidates | ||
|
|
||
| return [Candidate(CANDIDATE_IDS[candidate_id]) |
There was a problem hiding this comment.
list comprehensions:
https://www.smallsurething.com/list-dict-and-set-comprehensions-by-example/
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I would like you guys to review this implementation of STV. I will be happy to give you a code walkthrough in a personal or an online meeting.
Why Python
The proposed solution is to gradually port elections platform to Python, namely Django. There are much more people familiar with Python on campus, and it offers decent administration console out of the box.
As a first step, I am going to port create a baseline project, which will not be exposed outside but will allow us to perform some administrative tasks, such as creating/editing elections and calculating election results. Since fair implementation of STV was a big concern in Senate last year (which I totally understand), I would like you to review my implementation.
Why not existing implementation
code in
electionspyis overly verbose and is not very Pythonic. It doesn't follow many conventions, excessively uses classes, misses logging, and can be best described by a God-method pattern (Elections.compute_resultsis 240LOC + service classes and methods). After giving a try to make it up to standard, I found it will take less time to restart.Testing and debugging
All existing tests are included as doctests. Run:
python -m doctest stv.pyConcise debugging output is generated if logging is configured at
INFOlevel.DEBUGproduces even more verbose output. Simply config logging before calling stv:logging.basicConfig(level=logging.INFO)