include stem_count() as member function of Signature

[JohnAGoldsmith]

I put it in, but you will want to do it correctly...

[axanthos]

I've done this for stem_count, but you don't want to pass on this opportunity to learn how to do it correctly--and I'll leave doing this to affix_count() to you:

You had defined stem_count as follows:

def stem_count(self):
    return len(self.stems)

We know that our signatures are immutable, so there's no reason to compute this more than once. Rather we want the result of the first computation to be cached for later retrieval. To that effect, we delegate the computation to a new private method (by convention I just add the prefix "compute" to the name of the original method):

def _compute_stem_count(self):
    return len(self.stems)

Then we have the original method call the private one, and we decorate it as a "cached property" as follows:

@cached_property
def stem_count(self):
    return self._compute_stem_count()

As a result of these changes, because stem_count is now a property its value is simply obtained with sig.stem_count (no parentheses, I've changed lines 90 and 116 of graphics.py accordingly), and because it's cached it is computed only once, and simply retrieved from the cache afterwards.

For good measure I've also added a test for this in pycrab/test/test_signature.py:

def test_stem_count(self):
    self.assertEqual(self.signature.stem_count, 2)

Note that self.signature is already defined in the setUp() method of this file, which is called before each test:

def setUp(self):
    self.signature = morphology.Signature(
        stems=["want", "add", "add"],
        affixes=[morphology.NULL_AFFIX, "ed", "ing"],
    )

So the expected value of self.signature.stem_count is indeed 2. test_stem_count() serves both to verify that self.signature.stem_count returns the correct value and to document how it is used (without parentheses, in particular).

Voilà! You can try to do the same for affix_count, and don't forget docstrings ;)

[JohnAGoldsmith]

Great! thanks. John

On Mon, Dec 28, 2020 at 4:51 AM Aris Xanthos notifications@github.com wrote:

I've done this for stem_count, but you don't want to pass on this opportunity to learn how to do it correctly--and I'll leave doing this to affix_count() to you:

You had defined stem_count as follows:

def stem_count(self):

return len(self.stems)

We know that our signatures are immutable, so there's no reason to compute this more than once. Rather we want the result of the first computation to be cached for later retrieval. To that effect, we delegate the computation to a new private method (by convention I just add the prefix "compute" to the name of the original method):

def _compute_stem_count(self):

return len(self.stems)

Then we have the original method call the private one, and we decorate it as a "cached property" as follows:

@cached_property

def stem_count(self):

return self._compute_stem_count()

As a result of these changes, because stem_count is now a property its value is simply obtained with sig.stem_count (no parentheses, I've changed lines 90 and 116 of graphics.py accordingly), and because it's cached it is computed only once, and simply retrieved from the cache afterwards.

For good measure I've also added a test for this in pycrab/test/test_signature.py:

def test_stem_count(self):

self.assertEqual(self.signature.stem_count, 2)

Note that self.signature is already defined in the setUp() method of this file, which is called before each test:

def setUp(self):

self.signature = morphology.Signature(

    stems=["want", "add", "add"],

    affixes=[morphology.NULL_AFFIX, "ed", "ing"],

)

So the expected value of self.signature.stem_count is indeed 2. test_stem_count() serves both to verify that self.signature.stem_count returns the correct value and to document how it is used (without parentheses, in particular).

Voilà! You can try to do the same for affix_count, and don't forget docstrings ;)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/axanthos/pycrab/issues/10#issuecomment-751671443, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMKVF5FWW57NBELVEC4JYDSXBPJXANCNFSM4VKF4GLQ .