aclark4life / vanity

Get package download statistics from PyPI
GNU General Public License v2.0
66 stars 14 forks source link

Fix TODO: refactor normalize when given empty string #51

Closed hugovk closed 8 years ago

hugovk commented 8 years ago

test_empty has a TODO that needs fixing:

    def test_empty(self):
        """
        TODO: this test is rather slow to run,
        perhaps normalize could be refactored to check this
        and kick out faster.
        """
        normalized = vanity.normalize("")
        self.assertEqual(normalized, "")

So right now normalize is looking up on PyPI for a package called "" and not finding it. The empty string is a special case; there's no point looking for anything so it'd make more sense to validate and to return something sensible right away.

M3rs commented 8 years ago

That test shows that the PyPI search for "" is returning "". Could this be handled with just an if statement?

def normalize(name):
    # (docstring omitted)
    if name == "":
        return ""

    # ...other code
    return normalize_name
hugovk commented 8 years ago

Yes, that's one way.

M3rs commented 8 years ago

Would that be okay - Do you want me to work this into PR #50 ?

hugovk commented 8 years ago

Yes, that'd be okay, but put it in another PR: it's better to keep distinct things in each PR, it makes them easier to discuss and in general the simpler ones can get merged quicker.

M3rs commented 8 years ago

Good point, created PR #52 (also fix the test class name / remove the TODO)