rhasspy / gruut-ipa

Python library for manipulating pronunciations using the International Phonetic Alphabet (IPA)
MIT License
80 stars 12 forks source link

Make phones comparable and hashable #9

Open valericus opened 2 years ago

valericus commented 2 years ago

Hi there!

I'm trying to use your awesome library for some language investigations, and faced with a problem that phones are not comparable and not hashable. So here is a quick fix. If there is some better way to do this, kindly let me know, I'll adjust my PR.

msftcangoblowm commented 1 year ago

There are two files affected:

The code is trying to make a change to class Phone in __init__.py, rather than in phonemes.py

Even if applied to the correct file, neglected to modify the mro of class Phone

In phonemes.py from collections.abc import Hashable

class Phone: --> class Phone(Hashable):

In class method __hash__, should document what are the benefits of class Phone as a Hashable class In class method __eq__, should document it's a requirement of __hash__ method

Documenting helps later coders understand what things do. Which might be obvious to the original coder, but not necessary obvious to the next coder that comes along.

Hi! I'm Mr. the next coder and i had no idea the benefits of a Hashable object and had to look it up.

Recommend closing this PR without applying it. Then submit a new PR incorporating all the above suggestions

msftcangoblowm commented 1 year ago

@synesthesiam

Please close this PR without applying it

There is nothing wrong with the idea, however the implementation has: