-
Notifications
You must be signed in to change notification settings - Fork 47
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
refactor: implement pydantic schemas #80
base: main
Are you sure you want to change the base?
Changes from all commits
2deeb31
44ea751
2b0d038
583db9c
01e587b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2,6 +2,7 @@ | |||||||||||||||||||||
|
||||||||||||||||||||||
from fastapi import APIRouter | ||||||||||||||||||||||
|
||||||||||||||||||||||
from app.schemas import players as schemas | ||||||||||||||||||||||
from app.services.players.achievements import TransfermarktPlayerAchievements | ||||||||||||||||||||||
from app.services.players.injuries import TransfermarktPlayerInjuries | ||||||||||||||||||||||
from app.services.players.jersey_numbers import TransfermarktPlayerJerseyNumbers | ||||||||||||||||||||||
|
@@ -14,56 +15,56 @@ | |||||||||||||||||||||
router = APIRouter() | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
@router.get("/search/{player_name}") | ||||||||||||||||||||||
@router.get("/search/{player_name}", response_model=schemas.PlayerSearch, response_model_exclude_none=True) | ||||||||||||||||||||||
def search_players(player_name: str, page_number: Optional[int] = 1): | ||||||||||||||||||||||
tfmkt = TransfermarktPlayerSearch(query=player_name, page_number=page_number) | ||||||||||||||||||||||
found_players = tfmkt.search_players() | ||||||||||||||||||||||
return found_players | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
@router.get("/{player_id}/profile") | ||||||||||||||||||||||
@router.get("/{player_id}/profile", response_model=schemas.PlayerProfile, response_model_exclude_none=True) | ||||||||||||||||||||||
def get_player_profile(player_id: str): | ||||||||||||||||||||||
tfmkt = TransfermarktPlayerProfile(player_id=player_id) | ||||||||||||||||||||||
player_info = tfmkt.get_player_profile() | ||||||||||||||||||||||
return player_info | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
@router.get("/{player_id}/market_value") | ||||||||||||||||||||||
@router.get("/{player_id}/market_value", response_model=schemas.PlayerMarketValue, response_model_exclude_none=True) | ||||||||||||||||||||||
def get_player_market_value(player_id: str): | ||||||||||||||||||||||
tfmkt = TransfermarktPlayerMarketValue(player_id=player_id) | ||||||||||||||||||||||
player_market_value = tfmkt.get_player_market_value() | ||||||||||||||||||||||
return player_market_value | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
@router.get("/{player_id}/transfers") | ||||||||||||||||||||||
@router.get("/{player_id}/transfers", response_model=schemas.PlayerTransfers, response_model_exclude_none=True) | ||||||||||||||||||||||
def get_player_transfers(player_id: str): | ||||||||||||||||||||||
tfmkt = TransfermarktPlayerTransfers(player_id=player_id) | ||||||||||||||||||||||
player_market_value = tfmkt.get_player_transfers() | ||||||||||||||||||||||
return player_market_value | ||||||||||||||||||||||
Comment on lines
+39
to
43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix variable name inconsistency The variable name @router.get("/{player_id}/transfers", response_model=schemas.PlayerTransfers, response_model_exclude_none=True)
def get_player_transfers(player_id: str):
tfmkt = TransfermarktPlayerTransfers(player_id=player_id)
- player_market_value = tfmkt.get_player_transfers()
- return player_market_value
+ player_transfers = tfmkt.get_player_transfers()
+ return player_transfers 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
@router.get("/{player_id}/jersey_numbers") | ||||||||||||||||||||||
@router.get("/{player_id}/jersey_numbers", response_model=schemas.PlayerJerseyNumbers, response_model_exclude_none=True) | ||||||||||||||||||||||
def get_player_jersey_numbers(player_id: str): | ||||||||||||||||||||||
tfmkt = TransfermarktPlayerJerseyNumbers(player_id=player_id) | ||||||||||||||||||||||
player_jerseynumbers = tfmkt.get_player_jersey_numbers() | ||||||||||||||||||||||
return player_jerseynumbers | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
@router.get("/{player_id}/stats") | ||||||||||||||||||||||
@router.get("/{player_id}/stats", response_model=schemas.PlayerStats, response_model_exclude_none=True) | ||||||||||||||||||||||
def get_player_stats(player_id: str): | ||||||||||||||||||||||
tfmkt = TransfermarktPlayerStats(player_id=player_id) | ||||||||||||||||||||||
player_stats = tfmkt.get_player_stats() | ||||||||||||||||||||||
return player_stats | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
@router.get("/{player_id}/injuries") | ||||||||||||||||||||||
@router.get("/{player_id}/injuries", response_model=schemas.PlayerInjuries, response_model_exclude_none=True) | ||||||||||||||||||||||
def get_player_injuries(player_id: str, page_number: Optional[int] = 1): | ||||||||||||||||||||||
tfmkt = TransfermarktPlayerInjuries(player_id=player_id, page_number=page_number) | ||||||||||||||||||||||
players_injuries = tfmkt.get_player_injuries() | ||||||||||||||||||||||
return players_injuries | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
||||||||||||||||||||||
@router.get("/{player_id}/achievements") | ||||||||||||||||||||||
@router.get("/{player_id}/achievements", response_model=schemas.PlayerAchievements, response_model_exclude_none=True) | ||||||||||||||||||||||
def get_player_achievements(player_id: str): | ||||||||||||||||||||||
tfmkt = TransfermarktPlayerAchievements(player_id=player_id) | ||||||||||||||||||||||
player_achievements = tfmkt.get_player_achievements() | ||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,82 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from datetime import datetime | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from dateutil import parser | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from pydantic import BaseModel, ConfigDict, Field, field_validator | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from pydantic.alias_generators import to_camel | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class IDMixin(BaseModel): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
id: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class AuditMixin(BaseModel): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
updated_at: datetime = Field(default_factory=datetime.now) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class TransfermarktBaseModel(BaseModel): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
model_config = ConfigDict(alias_generator=to_camel) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@field_validator( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"date_of_birth", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"joined_on", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"contract", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"founded_on", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"members_date", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"from_date", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"until_date", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"date", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"contract_expires", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"joined", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"retired_since", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
mode="before", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
check_fields=False, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def parse_str_to_date(cls, v: str): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return parser.parse(v).date() if v else None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
except parser.ParserError: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+35
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix exception handling in The Apply this diff to fix the exception handling: def parse_str_to_date(cls, v: str):
try:
return parser.parse(v).date() if v else None
- except parser.ParserError:
+ except ValueError:
return None 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@field_validator( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"current_market_value", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"current_transfer_record", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"market_value", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"mean_market_value", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"members", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"total_market_value", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"age", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"goals", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"assists", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"yellow_cards", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"red_cards", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"minutes_played", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
mode="before", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
check_fields=False, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def parse_str_to_int(cls, v: str): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
parsed_value = ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
v.replace("bn", "000000000") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.replace("m", "000000") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.replace("k", "000") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.replace("€", "") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.replace(".", "") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.replace("+", "") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.replace(".", "") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.replace("'", "") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if v and any(char.isdigit() for char in v) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
else None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return int(parsed_value) if parsed_value else None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+57
to
+71
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve parsing of decimal values in The current implementation may incorrectly parse values with decimal points (e.g., Apply this diff to improve the parsing logic: def parse_str_to_int(cls, v: str):
parsed_value = (
(
v.replace("bn", "e9")
.replace("m", "e6")
.replace("k", "e3")
.replace("€", "")
.replace("+", "")
.replace("'", "")
)
if v and any(char.isdigit() for char in v)
else None
)
- return int(parsed_value) if parsed_value else None
+ return int(float(parsed_value)) if parsed_value else None This approach uses scientific notation to handle decimal values accurately. 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@field_validator("height", mode="before", check_fields=False) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def parse_height(cls, v: str): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if not any(char.isdigit() for char in v): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return int(v.replace(",", "").replace("m", "")) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+75
to
+77
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle decimal values in Converting height strings like Apply this diff to fix the conversion: def parse_height(cls, v: str):
if not any(char.isdigit() for char in v):
return None
- return int(v.replace(",", "").replace("m", ""))
+ return float(v.replace(",", "").replace("m", "")) 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@field_validator("days", mode="before", check_fields=False) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def parse_days(cls, v: str): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
days = "".join(filter(str.isdigit, v)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return int(days) if days else None |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
from app.schemas.clubs.players import ClubPlayers as ClubPlayers | ||
from app.schemas.clubs.profile import ClubProfile as ClubProfile | ||
from app.schemas.clubs.search import ClubSearch as ClubSearch |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,25 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from datetime import date | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from typing import Optional | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from app.schemas.base import TransfermarktBaseModel | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class ClubPlayer(TransfermarktBaseModel): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
name: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
position: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
date_of_birth: date | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
age: int | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
nationality: list[str] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
current_club: Optional[str] = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
height: Optional[int] = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
foot: Optional[str] = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
joined_on: Optional[date] = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
joined: Optional[str] = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
signed_from: Optional[str] = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
contract: Optional[date] = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
market_value: Optional[int] = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
status: Optional[str] = "" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+7
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add field constraints and documentation. The schema would benefit from:
Consider applying these improvements: class ClubPlayer(TransfermarktBaseModel):
- name: str
- position: str
- date_of_birth: date
- age: int
- nationality: list[str]
- current_club: Optional[str] = None
- height: Optional[int] = None
- foot: Optional[str] = None
- joined_on: Optional[date] = None
- joined: Optional[str] = None
- signed_from: Optional[str] = None
- contract: Optional[date] = None
- market_value: Optional[int] = None
- status: Optional[str] = ""
+ name: str = Field(..., description="Player's full name")
+ position: str = Field(..., description="Player's position on the field",
+ regex="^(Goalkeeper|Defender|Midfielder|Forward)$")
+ date_of_birth: date = Field(..., description="Player's date of birth")
+ age: int = Field(..., description="Player's age", ge=0, le=100)
+ nationality: list[str] = Field(..., min_items=1, description="List of player's nationalities")
+ current_club: Optional[str] = Field(None, description="Current club name")
+ height: Optional[int] = Field(None, description="Player's height in centimeters", gt=0, lt=300)
+ foot: Optional[str] = Field(None, description="Player's preferred foot",
+ regex="^(Left|Right|Both)$")
+ joined_on: Optional[date] = Field(None, description="Date when player joined the club")
+ joined: Optional[str] = Field(None, description="Season or transfer window when player joined")
+ signed_from: Optional[str] = Field(None, description="Previous club name")
+ contract: Optional[date] = Field(None, description="Contract end date")
+ market_value: Optional[int] = Field(None, description="Market value in euros")
+ status: Optional[str] = Field(None, description="Player's current status") 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class ClubPlayers(TransfermarktBaseModel): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
players: list[ClubPlayer] |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,47 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from datetime import date | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from typing import Optional | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from app.schemas.base import TransfermarktBaseModel | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class ClubSquad(TransfermarktBaseModel): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
size: int | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
average_age: float | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
foreigners: int | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
national_team_players: int | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+7
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add field validation and documentation The schema should include:
Consider applying these improvements: class ClubSquad(TransfermarktBaseModel):
+ """Represents the squad statistics of a club."""
- size: int
- average_age: float
- foreigners: int
- national_team_players: int
+ size: int = Field(ge=0, description="Total number of players in the squad")
+ average_age: float = Field(ge=0, description="Average age of all squad players")
+ foreigners: int = Field(ge=0, description="Number of foreign players in the squad")
+ national_team_players: int = Field(ge=0, description="Number of national team players in the squad") 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class ClubLeague(TransfermarktBaseModel): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
id: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
name: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
country_id: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
country_name: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
tier: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class ClubProfile(TransfermarktBaseModel): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
url: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
name: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
official_name: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
image: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
legal_form: Optional[str] = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
address_line_1: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
address_line_2: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
address_line_3: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
tel: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fax: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
website: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
founded_on: date | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
members: Optional[int] = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
members_date: Optional[date] = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
other_sports: Optional[list[str]] = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
colors: Optional[list[str]] = [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
stadium_name: str | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
stadium_seats: int | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
current_transfer_record: int | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
current_market_value: int | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
confederation: Optional[str] = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fifa_world_ranking: Optional[str] = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
squad: ClubSquad | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
league: ClubLeague | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
historical_crests: Optional[list[str]] = [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+22
to
+47
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix inconsistent defaults and add field validation There are several areas that need attention:
Apply these fixes: class ClubProfile(TransfermarktBaseModel):
+ """Represents detailed profile information of a club."""
url: str
- name: str
- official_name: str
- image: str
+ name: str = Field(min_length=1, description="Common name of the club")
+ official_name: str = Field(min_length=1, description="Legal/official name of the club")
+ image: HttpUrl = Field(description="URL to the club's primary crest/logo")
legal_form: Optional[str] = None
address_line_1: str
address_line_2: str
address_line_3: str
- tel: str
- fax: str
- website: str
+ tel: str = Field(pattern=r'^\+?[\d\s-]+$', description="Contact telephone number")
+ fax: str = Field(pattern=r'^\+?[\d\s-]+$', description="Contact fax number")
+ website: HttpUrl = Field(description="Official club website URL")
founded_on: date
members: Optional[int] = None
members_date: Optional[date] = None
- other_sports: Optional[list[str]] = None
- colors: Optional[list[str]] = []
+ other_sports: Optional[list[str]] = Field(default_factory=list, description="List of other sports departments")
+ colors: Optional[list[str]] = Field(default_factory=list, description="Club's official colors")
stadium_name: str
- stadium_seats: int
- current_transfer_record: int
- current_market_value: int
+ stadium_seats: int = Field(ge=0, description="Stadium capacity")
+ current_transfer_record: int = Field(description="Highest transfer fee (can be negative for sales)")
+ current_market_value: int = Field(ge=0, description="Current total market value of the club")
confederation: Optional[str] = None
fifa_world_ranking: Optional[str] = None
squad: ClubSquad
league: ClubLeague
- historical_crests: Optional[list[str]] = []
+ historical_crests: Optional[list[HttpUrl]] = Field(default_factory=list, description="URLs to historical club crests") Don't forget to add the necessary import: from datetime import date
from typing import Optional
+from pydantic import Field, HttpUrl
from app.schemas.base import TransfermarktBaseModel 📝 Committable suggestion
Suggested change
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
from typing import Optional | ||
|
||
from app.schemas.base import AuditMixin, IDMixin, TransfermarktBaseModel | ||
|
||
|
||
class ClubSearchResult(TransfermarktBaseModel, IDMixin): | ||
url: str | ||
name: str | ||
country: str | ||
squad: int | ||
market_value: Optional[int] = None | ||
|
||
|
||
class ClubSearch(TransfermarktBaseModel, AuditMixin): | ||
query: str | ||
page_number: int | ||
last_page_number: int | ||
results: list[ClubSearchResult] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
from app.schemas.competitions.clubs import CompetitionClubs as CompetitionClubs | ||
from app.schemas.competitions.search import CompetitionSearch as CompetitionSearch |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,12 @@ | ||||||||||||||||||||||||||||||||||||||||||
from app.schemas.base import AuditMixin, IDMixin, TransfermarktBaseModel | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
class CompetitionClub(TransfermarktBaseModel, IDMixin): | ||||||||||||||||||||||||||||||||||||||||||
name: str | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
class CompetitionClubs(TransfermarktBaseModel, IDMixin, AuditMixin): | ||||||||||||||||||||||||||||||||||||||||||
id: str | ||||||||||||||||||||||||||||||||||||||||||
name: str | ||||||||||||||||||||||||||||||||||||||||||
season_id: str | ||||||||||||||||||||||||||||||||||||||||||
clubs: list[CompetitionClub] | ||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+8
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Remove redundant ID field and enhance field validations.
class CompetitionClubs(TransfermarktBaseModel, IDMixin, AuditMixin):
+ """Represents a collection of clubs participating in a competition for a specific season."""
- id: str
- name: str
- season_id: str
- clubs: list[CompetitionClub]
+ name: str = Field(
+ min_length=1,
+ max_length=100,
+ description="The name of the competition"
+ )
+ season_id: str = Field(
+ regex="^[0-9]{4}$",
+ description="The season year identifier"
+ )
+ clubs: list[CompetitionClub] = Field(
+ min_items=1,
+ description="List of clubs participating in the competition"
+ ) 📝 Committable suggestion
Suggested change
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
from typing import Optional | ||
|
||
from app.schemas.base import AuditMixin, IDMixin, TransfermarktBaseModel | ||
|
||
|
||
class CompetitionSearchResult(TransfermarktBaseModel, IDMixin): | ||
name: str | ||
country: str | ||
clubs: int | ||
players: int | ||
total_market_value: Optional[int] = None | ||
mean_market_value: Optional[int] = None | ||
continent: Optional[str] = None | ||
|
||
|
||
class CompetitionSearch(TransfermarktBaseModel, AuditMixin): | ||
query: str | ||
page_number: int | ||
last_page_number: int | ||
results: list[CompetitionSearchResult] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
from app.schemas.players.achievements import PlayerAchievements as PlayerAchievements | ||
from app.schemas.players.injuries import PlayerInjuries as PlayerInjuries | ||
from app.schemas.players.jersey_numbers import PlayerJerseyNumbers as PlayerJerseyNumbers | ||
from app.schemas.players.market_value import PlayerMarketValue as PlayerMarketValue | ||
from app.schemas.players.profile import PlayerProfile as PlayerProfile | ||
from app.schemas.players.search import PlayerSearch as PlayerSearch | ||
from app.schemas.players.stats import PlayerStats as PlayerStats | ||
from app.schemas.players.transfers import PlayerTransfers as PlayerTransfers |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
from typing import Optional | ||
|
||
from app.schemas.base import AuditMixin, IDMixin, TransfermarktBaseModel | ||
|
||
|
||
class AchievementDetail(TransfermarktBaseModel): | ||
id: Optional[str] = None | ||
name: str | ||
|
||
|
||
class AchievementDetails(TransfermarktBaseModel): | ||
competition: Optional[AchievementDetail] = None | ||
season: AchievementDetail | ||
club: Optional[AchievementDetail] = None | ||
|
||
|
||
class PlayerAchievement(TransfermarktBaseModel): | ||
title: str | ||
count: int | ||
details: list[AchievementDetails] | ||
|
||
|
||
class PlayerAchievements(TransfermarktBaseModel, IDMixin, AuditMixin): | ||
achievements: list[PlayerAchievement] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Based on the gathered information, I can now provide the final response:
Schema structure matches the response but needs field name adjustments
The ClubSearch schema and actual response have slight field name mismatches:
pageNumber
but schema expectspage_number
lastPageNumber
but schema expectslast_page_number
These inconsistencies need to be fixed either in the schema or the service response to ensure proper validation.
🔗 Analysis chain
Verify schema compatibility with the search response
The response model configuration looks good, but let's verify the schema structure matches the actual response.
Let me gather more information about the actual response structure from TransfermarktClubSearch.
Let me check one more thing to verify the actual structure of the response being returned.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 905
Script:
Length of output: 1177
Script:
Length of output: 917