-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Undervote check #51
base: main
Are you sure you want to change the base?
Undervote check #51
Conversation
…idate spaces a nd returns a truncated ballot
Lets remove the undervote_function.py file since that shouldn't end up in the main repo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small tweak and then this is good!
src/votekit/cleaning.py
Outdated
|
||
def undervote(ballot: Ballot) -> Ballot: | ||
rank_list = ballot.ranking | ||
cleaned_rank_list = [rank for rank in rank_list if " " not in rank] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the parser defaults empty cells (aka undervotes) as None
so lets replace '' ''
with None
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it
@jamesturk hey these are my pull requests. I am not listed as a contributor, so I am unable to formally tag you-- I hope this will do! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
poetry.lock
can be removed from this repository (and added to .gitignore) since it is a library, pyproject.toml
is all you'll need for distributing this & it will save everyone's commits from having spurious lockfile updates
Ballot: a truncated ballot without empty slots | ||
""" | ||
|
||
def undervote(ballot: Ballot) -> Ballot: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should not be a closure (inner function) since it does not use any of the containing scope
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by no inner function, do you mean just left tab the undervote function and returning a ballot? something like this (sorry if this is ugly to read):
def undervote_profile(pp: PreferenceProfile) -> PreferenceProfile:
"""
takes a ballot and truncates its rankings in the case.
Args:
ballot (Ballot): a ballot with empty slots in a particular ranking list
Returns:
Ballot: a truncated ballot without empty slots
"""
def undervote(ballot: Ballot) -> Ballot:
rank_list = ballot.ranking
cleaned_rank_list = [rank for rank in rank_list if None not in rank]
return Ballot(
id=ballot.id,
ranking=cleaned_rank_list,
weight=Fraction(ballot.weight),
voters=ballot.voters,
)
# pp_clean = _clean(pp=pp, clean_ballot_func=undervote)
# return pp_clean
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try editing your comment to use ``` around code, as is it got pretty mangled.
from votekit.election_types import STV, fractional_transfer | ||
|
||
|
||
fpath = "/Users/emariedelanuez/VoteKit/src/votekit/mn_clean_ballots.csv" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there should not be hard-coded paths, or global variables like this in the library
fpath = "/Users/emariedelanuez/VoteKit/src/votekit/mn_clean_ballots.csv" | ||
|
||
|
||
def loadin(fpath): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this function doesn't do anything
return data | ||
|
||
|
||
data = loadin(fpath) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
global variables like this should be removed (especially ones w/ names that mask parameters like data
)
return swapcount | ||
|
||
|
||
rankinglist, candidates = stv_mini(data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this code belongs in a test file or similar, there should be no code at the top level of a file intended to be used as a library
""" | ||
n = len(list1) | ||
indices = list(range(n)) | ||
model_dictionary = dict(zip(list1, indices)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be written much more efficiently using enumerate
https://realpython.com/python-enumerate/
and returns a swap distance value.\n | ||
""" | ||
swapcount = 0 | ||
for index_i in range(len(unorderedlist)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above note about enumerate
this is the undervote code that takes in a ballot with empty candidate spaces and outputs a truncated ballot