Equal-Vote / starpy

Python implementation of the STAR Voting system
https://www.starvoting.org/
BSD 3-Clause "New" or "Revised" License
8 stars 4 forks source link

Bug fix in ties #17

Closed mikefranze closed 2 years ago

mikefranze commented 2 years ago

When resolving ties we're extending the runoff candidates list with the tie winner. extend expects a list, so when the candidate names are strings it would break up the string.

Instead of ['Allison', 'Bill'] the runoff candidates would be ['Allison', 'B', 'i', 'l', 'l'] which causes the error later on found in #16

endolith commented 2 years ago

This fixes the first example, but not the others. Should I file a second bug or just list them here?

        columns = ['Allison', 'Bill', 'Carmen', 'Doug']
        election = [[5, 4, 1, 4],
                    [5, 4, 1, 4],
                    [2, 4, 1, 2],
                    [4, 3, 2, 1],
                    [0, 5, 4, 4],
                    [3, 2, 4, 2],
                    [3, 1, 5, 3],
                    [3, 1, 5, 3],
                    [1, 3, 2, 2],
                    [4, 3, 5, 5]]
        ballots = pd.DataFrame(columns=columns, data=election)
        results = STAR(ballots)

        columns = ['Allison', 'Bill', 'Carmen', 'Doug']
        election = [[5, 4, 1, 4],
                    [5, 4, 1, 4],
                    [2, 4, 1, 2],
                    [4, 3, 2, 1],
                    [0, 5, 4, 4],
                    [3, 2, 4, 2],
                    [3, 1, 5, 3],
                    [3, 1, 5, 3],
                    [1, 3, 2, 2],
                    [4, 3, 5, 5]]
        ballots = pd.DataFrame(columns=columns, data=election)
        results = STAR(ballots)

both produce:

STAR.py", line 216, in STAR
    results['elected'].extend(round_results['winners'])

TypeError: 'NoneType' object is not iterable
mikefranze commented 2 years ago

Other bug fixed in latest commits.

endolith commented 2 years ago

Would it make more sense to return sets instead of lists? For things like 'runoff_candidates', since the order doesn't matter?

When writing tests, for instance, 'runoff_candidates': [0, 1] doesn't match [1, 0]

mikefranze commented 2 years ago

Correction on last commit: some test assertions were commented out and I also found a weird bug.

If I run STAR_Test.py, all tests pass. But if I run pytest, 2 tests fail. It looks like the two ways of calling pytest result in different outputs. using pytest {'elected': [0], 'round_results': [{'winners': [0], 'runner_up': 1, 'logs': [{'top_score': [1]}, {'top_score': [0]}, {'runoff_candidates': [1, 0]}]}]}

using STAR_Test.py {'elected': [1], 'round_results': [{'winners': [0], 'runner_up': 1, 'logs': [{'top_score': [1]}, {'top_score': [0]}, {'runoff_candidates': [1, 0]}]}]}

endolith commented 2 years ago

If you start a new kernel and run STAR_Test.py again does it pass?

https://github.com/pytest-dev/pytest/issues/3143#issuecomment-1207273459

Oh wait no you said pytest fails but running the file doesn't. Not sure why that would happen, will look later

endolith commented 2 years ago

When I checkout 666aea1c I don't see any failures, just 7 test passing, whether running STAR_Test.py directly or running pytest from command line.

mikefranze commented 2 years ago

So I tracked it down to the scores.argmax() line. Running the test the two different ways resulted in different outputs. I updated to not use it and make code more readable.

endolith commented 2 years ago

That's strange, why would it behave differently when run different ways?