Filtering data stored using classes via user-called function the proper way?

Question

I'm currently working on an API package. Basically what it does is scrapes team data and stores them in list of Team objects. The team object contains data such as Team.name,rank, country etc.

If a user wants to filter that list to find something in it, I have this really messy and ugly (but working) function:

def find_team(self, name=None, country=None, rank_exactly=None, score_exactly=None, rank_change=None, team_id=None,
                  score_lower=None, score_higher=None, partial_name=None, rank_lower=None, rank_higher=None):
        """
        Finds a team by provided keyword attributes, returns a list of teams matching the filter settings
        Keyword Arguments:
        name - exact team name (case insensitive)
        country - team origin country
        rank_exactly - rank exactly
        rank_lower - only teams with lower rank than provided
        rank_higher - only teams with higher rank than provided
        rank_change - the state of ank change
        score_exactly - score exactly
        score_lower - only teams with lower score than provided
        score_higher - only teams with higher score than provided
        team_id = exact team id
        partial_name - part of name
        """
        if not self.teams:
            raise AttributeError("Object team list is empty")
        results = []
        for team in self.teams:
            all_args=[]
            if name:
                if name.lower() == team.name.lower():
                    all_args.append(True)
                else:
                    all_args.append(False)
            if country:
                if country.lower() == team.country.lower():
                    all_args.append(True)
                else:
                    all_args.append(False)
            if rank_exactly:
                if rank_exactly == team.rank:
                    all_args.append(True)
                else:
                    all_args.append(False)
            if rank_higher:
                if int(rank_higher) > int(team.rank):
                    all_args.append(True)
                else:
                    all_args.append(False)
            if rank_lower:
                if int(rank_lower) < int(team.rank):
                    all_args.append(True)
                else:
                    all_args.append(False)
            if score_exactly:
                if score_exactly == team.score:
                    all_args.append(True)
                else:
                    all_args.append(False)
            if rank_change:
                if rank_change == team.rank_change:
                    all_args.append(True)
                else:
                    all_args.append(False)
            if team_id:
                if team_id == team.team_id:
                    all_args.append(True)
                else:
                    all_args.append(False)
            if score_higher:
                if int(score_higher) < int(team.score.replace(',','')):
                    all_args.append(True)
                else:
                    all_args.append(False)
            if score_lower:
                if int(score_lower) > int(team.score.replace(',','')):
                    all_args.append(True)
                else:
                    all_args.append(False)
            if partial_name:
                if partial_name.lower() in team.name.lower():
                    all_args.append(True)
                else:
                    all_args.append(False)
            if all(all_args):
                results.append(team)
        return results

How could I make this more Pythonic and just better in general?

Answer 1

        if name:
            if name.lower() == team.name.lower():
                all_args.append(True)
            else:
                all_args.append(False)
        if country:
            if country.lower() == team.country.lower():
                all_args.append(True)
            else:
                all_args.append(False)
        if rank_exactly:
            if rank_exactly == team.rank:
                all_args.append(True)
            else:
                all_args.append(False)
        if rank_higher:
            if int(rank_higher) > int(team.rank):
                all_args.append(True)
            else:
                all_args.append(False)
        if rank_lower:
            if int(rank_lower) < int(team.rank):
                all_args.append(True)
            else:
                all_args.append(False)
        if score_exactly:
            if score_exactly == team.score:
                all_args.append(True)
            else:
                all_args.append(False)
        if rank_change:
            if rank_change == team.rank_change:
                all_args.append(True)
            else:
                all_args.append(False)
        if team_id:
            if team_id == team.team_id:
                all_args.append(True)
            else:
                all_args.append(False)
        if score_higher:
            if int(score_higher) < int(team.score.replace(',','')):
                all_args.append(True)
            else:
                all_args.append(False)
        if score_lower:
            if int(score_lower) > int(team.score.replace(',','')):
                all_args.append(True)
            else:
                all_args.append(False)
        if partial_name:
            if partial_name.lower() in team.name.lower():
                all_args.append(True)
            else:
                all_args.append(False)

You're being needlessly specific with your if statements here. If you do something like this...

        if name:
            all_args.append(name.lower() == team.name.lower())
        if country:
             all_args.append(country.lower() == team.country.lower())

and so on, you'd reduce the needless verbosity.

Answer 2

Because of the way you use all_args, there is no need to add True in it as it will not make any difference. We can them simplify the code by removing the corresponding branches and inversiong the conditions :

    for team in self.teams:
        all_args=[]
        if name:
            if name.lower() != team.name.lower():
                all_args.append(False)
        if country:
            if country.lower() != team.country.lower():
                all_args.append(False)
        if rank_exactly:
            if rank_exactly != team.rank:
                all_args.append(False)
        if rank_higher:
            if int(rank_higher) <= int(team.rank):
                all_args.append(False)
        if rank_lower:
            if int(rank_lower) >= int(team.rank):
                all_args.append(False)
        if score_exactly:
            if score_exactly != team.score:
                all_args.append(True)
        if rank_change:
            if rank_change != team.rank_change:
                all_args.append(False)
        if team_id:
            if team_id != team.team_id:
                all_args.append(False)
        if score_higher:
            if int(score_higher) >= int(team.score.replace(',','')):
                all_args.append(False)
        if score_lower:
            if int(score_lower) <= int(team.score.replace(',','')):
                all_args.append(False)
        if partial_name:
            if partial_name.lower() not in team.name.lower():
                all_args.append(False)
        if all(all_args):
            results.append(team)

(I might have introduced typos but you see the spirit)

Now, you can remove the non-required level of nesting :

        all_args=[]
        if name and name.lower() != team.name.lower():
            all_args.append(False)
        if country and country.lower() != team.country.lower():
            all_args.append(False)
        if rank_exactly and rank_exactly != team.rank:
            all_args.append(False)
        if rank_higher and int(rank_higher) <= int(team.rank):
            all_args.append(False)
        if rank_lower and int(rank_lower) >= int(team.rank):
            all_args.append(False)
        if score_exactly and score_exactly != team.score:
            all_args.append(True)
        if rank_change and rank_change != team.rank_change:
            all_args.append(False)
        if team_id and team_id != team.team_id:
            all_args.append(False)
        if score_higher and int(score_higher) >= int(team.score.replace(',','')):
            all_args.append(False)
        if score_lower and int(score_lower) <= int(team.score.replace(',','')):
            all_args.append(False)
        if partial_name and partial_name.lower() not in team.name.lower():
            all_args.append(False)
        if all(all_args):
            results.append(team)

Now, because a single False is enough for the final all to fail, we can add else between conditions in order to stop at first failing conditions.

        all_args=[]
        if name and name.lower() != team.name.lower():
            all_args.append(False)
        elif country and country.lower() != team.country.lower():
            all_args.append(False)
        elif rank_exactly and rank_exactly != team.rank:
            all_args.append(False)
        elif rank_higher and int(rank_higher) <= int(team.rank):
            all_args.append(False)
        elif rank_lower and int(rank_lower) >= int(team.rank):
            all_args.append(False)
        elif score_exactly and score_exactly != team.score:
            all_args.append(True)
        elif rank_change and rank_change != team.rank_change:
            all_args.append(False)
        elif team_id and team_id != team.team_id:
            all_args.append(False)
        elif score_higher and int(score_higher) >= int(team.score.replace(',','')):
            all_args.append(False)
        elif score_lower and int(score_lower) <= int(team.score.replace(',','')):
            all_args.append(False)
        elif partial_name and partial_name.lower() not in team.name.lower():
            all_args.append(False)
        if all(all_args):
            results.append(team)

Wait, once this is done, there is no need for the all_args list at all ? If we transform the last if in an elif, we are good to go. This removes the need for adding elements to all_args :

        if name and name.lower() != team.name.lower():
            pass
        elif country and country.lower() != team.country.lower():
            pass
        elif rank_exactly and rank_exactly != team.rank:
            pass
        elif rank_higher and int(rank_higher) <= int(team.rank):
            pass
        elif rank_lower and int(rank_lower) >= int(team.rank):
            pass
        elif score_exactly and score_exactly != team.score:
            pass
        elif rank_change and rank_change != team.rank_change:
            pass
        elif team_id and team_id != team.team_id:
            pass
        elif score_higher and int(score_higher) >= int(team.score.replace(',','')):
            pass
        elif score_lower and int(score_lower) <= int(team.score.replace(',','')):
            pass
        elif partial_name and partial_name.lower() not in team.name.lower():
            pass
        else:
            results.append(team)

It is the time I realize that I shouldn't have inverted all conditions at the very beginning as the whole thing can be rewritten :

        if  (not name or name.lower() == team.name.lower()) and \
            (not country or country.lower() == team.country.lower()) and \
            (not rank_exactly or rank_exactly == team.rank) and \
            (not rank_higher or int(rank_higher) > int(team.rank)) and \
            (not rank_lower or int(rank_lower) < int(team.rank)) and \
            (not score_exactly or score_exactly == team.score) and \
            (not rank_change or rank_change == team.rank_change) and \
            (not team_id or team_id == team.team_id) and \
            (not score_higher or int(score_higher) < int(team.score.replace(',',''))) and \
            (not score_lower or int(score_lower) > int(team.score.replace(',',''))) and \
            (not partial_name or partial_name.lower() in team.name.lower()):
            results.append(team)

If you like it that way, you can write this in a list comprehension :

    if not self.teams:
        raise AttributeError("Object team list is empty")
    return [tem for team in self.teams 
        if  (not name or name.lower() == team.name.lower()) and \
            (not country or country.lower() == team.country.lower()) and \
            (not rank_exactly or rank_exactly == team.rank) and \
            (not rank_higher or int(rank_higher) > int(team.rank)) and \
            (not rank_lower or int(rank_lower) < int(team.rank)) and \
            (not score_exactly or score_exactly == team.score) and \
            (not rank_change or rank_change == team.rank_change) and \
            (not team_id or team_id == team.team_id) and \
            (not score_higher or int(score_higher) < int(team.score.replace(',',''))) and \
            (not score_lower or int(score_lower) > int(team.score.replace(',',''))) and \
            (not partial_name or partial_name.lower() in team.name.lower())]

Now, trying to improve things :

• you should probably include the massive condition in a function or something on its own.
• i don't see a real need to raise an exception if the list is initially empty : it seems to be valid to look for something in an empty list... and not find it.
• I think you should be using is None/is not None instead of just the conversion to boolean. Indeed, if you look for a team whose score is exactly 0, your filter will be ignored.