Refactor Players Search parsing #55
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe recent updates involve a refactoring of the player search functionality, enhancing the data structure to include player nationalities and improving naming consistency. Utility functions have been updated to handle new parameters and ensure robust text processing. XPath expressions have been redefined to adapt to these changes. Additionally, the testing suite has been adjusted to accommodate the handling of negative integers and the updated structure for player nationalities. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (5)
- app/services/players/search.py (2 hunks)
- app/utils/utils.py (5 hunks)
- app/utils/xpath.py (1 hunks)
- tests/conftest.py (1 hunks)
- tests/players/test_players_search.py (1 hunks)
Additional comments: 6
tests/players/test_players_search.py (1)
- 46-46: The update to the test schema to expect a list of nationalities instead of a single string is correct and aligns with the changes made in the PR.
app/services/players/search.py (2)
42-42: Please clarify the usage of
self.page.xpath. Thexpathmethod is not a standard method on ElementTree objects. Ifself.pageis an ElementTree object, you should useElementTree.findall()or similar methods provided by ElementTree for XPath expressions.45-66: The changes to the
__parse_search_resultsmethod correctly reflect the updated data structure and naming convention for the player search results.app/utils/utils.py (2)
- 48-56: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [40-64]
The changes to the
extract_from_urlfunction correctly handle the new optionaltfmkt_urlparameter by returningNoneif it is not provided.
- 95-105: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [86-102]
The changes to the
safe_regexfunction correctly handle the newtextparameter type and ensure safe processing by checking iftextis a string or a list and not empty before proceeding.app/utils/xpath.py (1)
- 47-56: The changes to the
Searchnested class, including the removal of theFOUNDattribute and the addition of new attributesRESULTSandNATIONALITIES, correctly reflect the updated XPath expressions and align with the PR objectives.
| @@ -34,7 +34,7 @@ def regex_value_variation(): | |||
|
|
|||
| @pytest.fixture | |||
| def regex_integer(): | |||
| return Regex(r"^\d+$") | |||
| return Regex(r"^(\d+|-)$") | |||
There was a problem hiding this comment.
The regex pattern for matching integers is incorrect. It currently matches a single hyphen or a sequence of digits, but it does not match negative integers. Here's a corrected version:
- return Regex(r"^(\d+|-)$")
+ return Regex(r"^-?\d+$")This pattern will match any integer, positive or negative, as intended.
Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| return Regex(r"^(\d+|-)$") | |
| return Regex(r"^-?\d+$") |
Summary by CodeRabbit