mggg / VoteKit

A Swiss Army Knife for computational social choice research
https://votekit.readthedocs.io/en/latest/
MIT License
10 stars 12 forks source link

Cumulative voting #106

Closed cdonnay closed 7 months ago

cdonnay commented 9 months ago

This PR adds functionality for points based ballots. I felt like this deserved a new class, as it really is distinct from full rankings.

@jamesturk @drdeford @jgibson517 @jennjwang @ziglaser

cdonnay commented 9 months ago

Looks like creating this new PointBallot class has thrown a lot of type errors in mypy. Would be happy to chat about the best way to address that!

jgibson517 commented 9 months ago

Hi @cdonnay, I view the Ballot object as more of data structure then an object that dictates what type of ranking system is used. Instead I might lean towards adding an optional points attribute to the existing class.

A couple thoughts here: When we first designed the class, I think we didn't think users would really need to touch it much, but I don't think that's really the case anymore. So I think if we give a little more flexibility in how we initialize a Ballot, we won't need a second class.

I think an optional points method in the form of dictionary or vector can live in Ballot class. I also like in your PointBallot you default the weights to 1 if a user does not specify, we should adopt that into the Ballot object. Since the rest of ballot functionality is not exposed to the user (i.e users don't adjust ballot attributes when running elections), I think its more streamlined to have a single Python object do the work. Happy to chat about this more too, if helpful!!

jamesturk commented 9 months ago

re: mypy errors If this does go forward as two separate classes, PointBallot should probably inherit from Ballot which will help with the type issues but also break the current isinstance checks. It'd be preferred here to have the two share an explicit interface, which is part of what mypy is so upset about. Once that's true they can be used almost interchangably, relying on checking for PointBallot only when its customizations are needed.

cdonnay commented 9 months ago

Alright, let me take another go at this using just a Ballot class, and implementing some kind of point system within that.