You are viewing a single comment's thread from:

RE: Introducing Understat, a Python package for revolutionary football metrics

in #understat7 years ago

It's cool to see you use asyncio instead of sync requests. Package look good with all the code/docstrings/unit tests. I have minor comments, some may be opinionated:

So instead of using an API, the way the data is retrieved is by scraping their website for <script>s, and using a regular expression to match the data we want.

Btw, by seeing the name "wrapper", I thought it was an API wrapper. :) Maybe calling it scraper or something like that may be better.

def find_match(scripts, pattern):
    """Returns the first match found in the given scripts."""

    for script in scripts:
        match = re.search(pattern, script.string)
        if match:
            break

    return match

This may be refactored into sth like that maybe? (This changes the behaviour slightly, though. Current impl. will throw
an error if it can't find a match, while the alternative one returns None if there are no matches)

def find_match(scripts, pattern):
    """Returns the first match found in the given scripts."""

    for script in scripts:
        match = re.search(pattern, script.string)
        if match:
            return match


Your contribution has been evaluated according to Utopian policies and guidelines, as well as a predefined set of questions pertaining to the category.

To view those questions and the relevant answers related to your post, click here.


Need help? Chat with us on Discord.

[utopian-moderator]

Sort:  

Thanks for the review, Emre! I think you are correct for both cases, so I'll change both for the next version.

Thank you for your review, @emrebeyler! Keep up the good work!

Coin Marketplace

STEEM 0.09
TRX 0.32
JST 0.033
BTC 108415.08
ETH 3847.10
USDT 1.00
SBD 0.61