Skip to content

Improved stable marriage problem Python implementation #687

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CONTRIBUTORS.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,4 @@ This file lists everyone, who contributed to this repo and wanted to show up her
- Vincent Zalzal
- Jonathan D B Van Schenck
- James Goytia
- Amaras
116 changes: 62 additions & 54 deletions contents/stable_marriage_problem/code/python/stable_marriage.py
Original file line number Diff line number Diff line change
@@ -1,25 +1,19 @@
# Submitted by Marius Becker
# Updated by Amaras


import sys
from random import shuffle
from copy import copy
from string import ascii_uppercase
from string import ascii_uppercase, ascii_lowercase

def main():
# Set this to however many men and women you want
if len(sys.argv) > 1:
num_pairs = int(sys.argv[1])
else:
num_pairs = 5

# There are only 26 possible names
if num_pairs > 13:
print('You can\' have more than 13 pairs.')
return
def main():
# Set this to however many men and women you want, up to 26
num_pairs = 5

# Create all Person objects
men = [ Person(name) for name in ascii_uppercase[0:num_pairs] ]
women = [ Person(name) for name in ascii_uppercase[num_pairs:num_pairs*2] ]
men = [Person(name) for name in ascii_uppercase[:num_pairs]]
women = [Person(name) for name in ascii_lowercase[:num_pairs]]

# Set everyone's preferences
for man in men:
Expand All @@ -31,62 +25,64 @@ def main():
shuffle(woman.preference)

# Run the algorithm
resolve(men, women)
stable_marriage(men, women)

# Print preferences and the result
print('Preferences of the men:')
for man in men:
print('{}: {}'.format(man.name, ', '.join([ p.name for p in man.preference ])))
print(man)

print()

print('Preferences of the women:')
for woman in women:
print('{}: {}'.format(woman.name, ', '.join([ p.name for p in woman.preference ])))
print(woman)

print('')
print('\n')

print('The algorithm gave this solution:')
for man in men:
print('{} + {}'.format(man.name, man.partner.name))
print(f'{man.name} + {man.partner.name}')

def resolve(men, women):

def stable_marriage(men, women):
"""Finds pairs with stable marriages"""
cont = True
while cont:

while True:
# Let every man without a partner propose to a woman
for man in men:
if not man.has_partner():
if not man.has_partner:
man.propose_to_next()

# Let the women pick their favorites
for woman in women:
woman.pick_preferred()

# Continue only when someone is still left without a partner
cont = False
for man in men:
if not man.has_partner():
cont = True
break
if all((man.has_partner for man in men)):
return


class Person:
name = None
preference = None
pref_index = 0
candidates = None
partner = None

def __init__(self, name):
self.name = name
self.preference = []
self.candidates = []
self.pref_index = 0
self._partner = None

def get_next_choice(self):
@property
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we were talking about these @property with @leios the other day and I think we came to the conclusion that it is more confusing than helpful.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are points both in favour and against properties.
The problem is that using getters/setters in Python is really not idiomatic, especially since all your attributes are basically public.
I think using properties yields cleaner code, but I know they are more confusing for beginners. I'm not sure I want to use getters and setters here, but I'll go with what's required when I'm told to.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll be honest, I am not sure what an @property is. Is this something that normal python programmers know? I know python (and have taught python classes for beginners), but have never seen this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A property is an intermediate-level concept in Python. It basically allows the access of some methods as instance attributes. They usually replace the more usual idiom get_x, set_x (plus delete_x when needed) in other languages.
Without getting much into my thought process, I divide instance "things" into two categories: nouns and verbs.

A noun should be accessed as instance.noun, and should not do many things when you access it, except maybe when setting it (for example in this case: self.next_choice, or self.has_partner are things I consider as nouns).
A verb should be accessed as instance.verb(), and it usually does something (relatively) unique, like the self.propose_to_next, or the self.pick_preferred methods.

Properties are something that advanced Python programmers use whenever they need a getter, getter/setter, or deleter access on an instance attribute, but need some kind of obfuscation, recalculation on access, validation, or custom deletion. They are very useful tools, but should not be used everywhere.

Hope it helps you, but I get why they are confusing for beginners with prior knowledge of other languages, as Python is kind of weird compared to C and descendants.

Copy link
Contributor

@jdonszelmann jdonszelmann Jul 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Property is pretty well known, but in my opinion a bit of a hack (from the language side) and especially new programmers might not know about. I don't deny there are occasions in which they're very useful, but I think the purpose of the AAA is to be as clear as possible, even to new programers. And that, this is not. Especially because I think (though Berquist doesn't agree with me I think) that it's easy to remove them here. The partner property can just be an access to the _partner (maybe rename to just partner) field, and has_partner really is just a None check which in my opinion is really easy to do where the method is used, or alternatively, a call to has_partner() as a function really isn't that bad either. Then we're left with next_choice which can honestly just be a function right? Just call .next_choice(). The only 'justified' use of properties here is the partner setter.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can agree for has_partner and next_choice, even though I think the code is cleaner with properties.
However, I'm not sure I can get the partner setter without the partner getter, so we have to have either both or neither.
Also, the partner property is just an access to _partner, just a bit more obfuscated, which I agree can be confusing.

I can remove the @property on has_partner (I can even remove the calls entirely and just inline it), and on next_choice, but I don't want to have a set_partner method call, which is really not Pythonic, instead of a simple self.partner = person.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot find an easy way to remove the partner setter either, so I guess that will be necessary. I guess this is one of those instances where as I said in the previous comment

there are occasions in which they're very useful

So I guess we will all just have to live with it :P

I am not quite sure if we then want to remove the other properties, they don't add too much but if we commit to using properties anyway they aren't as bad either.

def next_choice(self):
"""Return the next person in the own preference list"""
if self.pref_index >= len(self.preference):
try:
return self.preference[self.pref_index]
except IndexError:
return None

return self.preference[self.pref_index]

def propose_to_next(self):
"""Propose to the next person in the own preference list"""
person = self.get_next_choice()
person = self.next_choice
person.candidates.append(self)
self.pref_index += 1

Expand All @@ -99,33 +95,45 @@ def pick_preferred(self):
if person == self.partner:
break
elif person in self.candidates:
self.set_partner(person)
self.partner = person
break

# Rejected candidates don't get a second chance. :(
# Rejected candidates don't get a second chance
self.candidates.clear()

def get_partner(self):
"""Return the current partner"""
return self.partner
@property
def partner(self):
return self._partner

# The call self.partner = person sets self._partner as person
# However, since engagement is symmetrical, self._partner._partner
# (which is then person._partner) also needs to be set to self
@partner.setter
def partner(self, person):
"""Set a person as the new partner and sets the partner of that
person as well"""

def set_partner(self, person):
"""Set a person as the new partner and run set_partner() on that person
as well"""
# Do nothing if nothing would change
if person != self.partner:
if person != self._partner:
# Remove self from current partner
if self.partner is not None:
self.partner.partner = None
if self._partner is not None:
self._partner._partner = None

# Set own and the other person's partner
self.partner = person
if self.partner is not None:
self.partner.partner = self
self._partner = person
if self._partner is not None:
self._partner._partner = self

# This allows use of self.has_partner instead of self.has_partner()
@property
def has_partner(self):
"""Determine whether this person currently has a partner or not"""
return self.partner != None
"""Determine whether this person currently has a partner or not."""
return self.partner is not None

# This allows the preferences to be printed more elegantly
def __str__(self):
return f'{self.name}: {", ".join(p.name for p in self.preference)}'


if __name__ == '__main__':
main()