Improved stable marriage problem Python implementation

[Amaras]

I did several things with this code change:

1. Updated it to use Python 3.6+ f-strings for formatting
2. Rendered the code more PEP8-compliant (manual, might not be perfect)
3. Used properties instead of getters/setters for partner; and has_partner only really made sense if it could be a pseudo-attribute.
   (The following are minor updates)
4. Added possible pairs (using lowercase women and uppercase men, instead of all uppercase)
5. Made it a bit more fool-proof: you could call "python stable_marriage.py a" and it would crash. I made sure it defaults to 5 pairs if you pass either nothing or an invalid string, instead of a ValueError traceback.
6. (very minor) Changed the printing format and added my name as an updator + in CONTRIBUTORS.md

I think that's all

[berquist]

Thanks for your first contribution! I'm a fan of your changes. I've pointed out a few things that you can add which in my opinion make the output more readable.

Also, don't forget to fix the conflict in the contributors file.

[Amaras]

Thanks for the review. I sadly have something more urgent on my hands for now, so I will come back to it once I finally finish my school year.
In the meantime, I have to say I have built upon the previous code, which was not really good, so mine was bound to be a bit weird on the edges without a complete refactor.

[Amaras]

There we go, this should have addressed the changes requested for the review. I'm happy to get back to helping the AAA

[Liikt]

All in all i like the changes. There is just some syntactic sugar in there that gets confusing if you just want to read about the algorithm.

[Liikt]

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.

[Amaras]

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.

[leios]

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.

[Amaras]

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.

[jdonszelmann]

@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.

[Amaras]

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.

[jdonszelmann]

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.

[Liikt]

This can get confusing. If you set your own partner, logically you don't expect a change for your partner. Especially because the other one may reject you.

[Amaras]

I know this is implementation specific, I just rewrote the set_partner function to use a writable property.
The call self.partner = person is only used in the pick_preferred function, so the person already proposed to you, and you are accepting their proposal.
Also, being engaged is reciprocal, so your partner's partner is yourself, so you need to change up to all three of self._partner._partner, self._partner, and person._partner accordingly.

[berquist]

I am fine with the request to remove asking for input, but I disagree about the removal of properties. Of course it can be done without them, but this particular use of them is not bad and does lead to cleaner code in the main loop of the algorithm. I agree that setters and getters in Python are not appropriate.

[leios]

As this was approved by @berquist , I am happy enough merging this once the contributor's file is fixed.

[Amaras]

Should be good to go.

We've come to an agreement that properties are appropriate here.