Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implemented search.py and wrote basic tests #86

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 58 additions & 1 deletion fpl/fpl.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import os

import requests
from unidecode import unidecode

from .constants import API_URLS
from .models.classic_league import ClassicLeague
Expand All @@ -38,7 +39,8 @@
from .models.team import Team
from .models.user import User
from .utils import (average, fetch, get_current_user, logged_in,
position_converter, scale, team_converter)
position_converter, scale, team_converter,
levenshtein_distance)


class FPL:
Expand Down Expand Up @@ -289,6 +291,61 @@ async def get_players(self, player_ids=None, include_summary=False,

return players

async def search_players(self, player_name, num_players=1,
include_summary=False, return_json=False):
"""Returns the player(s) given input search term by using Levenshtein distance,
or the mininum number of edits to turn one string into the other. Specifically,
the distance is defined as the minimum of Levenshtein distances from search string
to player's full name (`first_name` and `last_name`) and player's `web_name`

:param str plauer_name: The player name to search on
:param int num_players: (optional) The number of players to return in the search
:param boolean include_summary: (optional) Includes a player's summary
if ``True``.
:param return_json: (optional) Boolean. If ``True`` returns a list of
``dict``s, if ``False`` returns a list of :class:`Player`
objects. Defaults to ``False``.
"""

players = getattr(self, "elements")
N = len(players)
search_term = player_name.lower()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't used for anything.

min_id = 0
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't used for anything.

min_ed = float('inf')
eds = {}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is eds?

player_ids = [0]*N
for i, player in enumerate(players.values()):
player_ids[i] = player['id']
full_name = unidecode(player['first_name'] + ' ' + player['second_name']).lower()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be something like

first_name = unidecode(player["first_name"])
second_name = unidecode(player["second_name"])
full_name = f"{first_name} {second_name}".lower()

web_name = unidecode(player['web_name']).lower()
ed = min(levenshtein_distance(full_name, player_name),
levenshtein_distance(web_name, player_name))
Comment on lines +325 to +326
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess search_term should be used here?


if ed < min_ed:
min_ed = ed
min_id = player['id']

eds[player['id']] = ed

player_ids.sort(key=lambda player_id: eds[player_id])

players_list = []
for i in range(num_players):
player = players[player_ids[i]]

if include_summary:
player_summary = await self.get_player_summary(
player["id"], return_json=True)
player.update(player_summary)

players_list.append(player)

if return_json:
return players_list
else:
return [Player(p, self.session) for p in players_list]


async def get_fixture(self, fixture_id, return_json=False):
"""Returns the fixture with the given ``fixture_id``.

Expand Down
16 changes: 16 additions & 0 deletions fpl/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,22 @@ def get_headers(referer):
"Referer": referer
}

def levenshtein_distance(s1, s2):
"""Returns Levenshtein distance of two strings.
"""
if len(s1) > len(s2):
s1, s2 = s2, s1

distances = range(len(s1) + 1)
for i2, c2 in enumerate(s2):
distances_ = [i2+1]
for i1, c1 in enumerate(s1):
if c1 == c2:
distances_.append(distances[i1])
else:
distances_.append(1 + min((distances[i1], distances[i1 + 1], distances_[-1])))
distances = distances_
return distances[-1]

async def get_current_user(session):
user = await fetch(session, API_URLS["me"])
Expand Down
20 changes: 20 additions & 0 deletions tests/test_fpl.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,26 @@ async def test_player_summaries(self, loop, fpl):
player_summaries = await fpl.get_player_summaries([1, 2, 3], True)
assert isinstance(player_summaries[0], dict)

async def test_search_players(self, loop, fpl):
# test search_players
players = await fpl.search_players('lucas moura')
assert isinstance(players[0], Player)
assert players[0].id == 345

players = await fpl.search_players('nicolas pepe')
assert isinstance(players[0], Player)
assert players[0].id == 488

players = await fpl.search_players('lucas', num_players=5)
assert all([isinstance(p, Player) for p in players])
assert len(players) == 5

players = await fpl.search_players('nicolas pepe', return_json=True)
assert isinstance(players[0], dict)

players_with_summary = await fpl.search_players('nicolas pepe', include_summary=True)
assert isinstance(players_with_summary[0].fixtures, list)

async def test_player(self, loop, fpl):
# test invalid ID
with pytest.raises(ValueError):
Expand Down
1 change: 1 addition & 0 deletions tests/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ async def test_init(self, loop):

async def test_get_gameweek_history_unknown_gameweek_cached(
self, loop, user):
print(user)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be removed.

history = await user.get_gameweek_history()
assert history is user._history["current"]
assert isinstance(history, list)
Expand Down