amosbastian / understat

An asynchronous Python package for https://understat.com/.
MIT License
157 stars 30 forks source link

Remove reliance on bs4 and regex to parse html #5

Closed rasoolsomji closed 3 years ago

rasoolsomji commented 5 years ago

Potential improvement for #4

It's hard to test if or how much it has sped up, I think my test suite is running fractionally faster, but all other online benchmark tools I've seen rate regex and BeautifulSoup as slower than the method I've used here.

All tests pass (except for test_get_league_fixtures_with_options() since there are no unplayed fixtures in the 2018 season)

amosbastian commented 5 years ago

Thanks, will check it out once I get back from holiday in roughly two weeks.

amosbastian commented 5 years ago

Sorry for taking so long getting back to you. Checking the CI and testing it locally I seem to get an error when trying it out:

json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

Any idea how to fix this / why you don't get this error?

rasoolsomji commented 5 years ago

No worries! I happen to be away myself now for the next couple of weeks but I'll take a look when I'm back

On Wed, 14 Aug 2019, 23:32 Amos Bastian, notifications@github.com wrote:

Sorry for taking so long getting back to you. Checking the CI and testing it locally I seem to get an error when trying it out:

json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/amosbastian/understat/pull/5?email_source=notifications&email_token=AD6FZCCFZ5S72FI6JQ2CQ53QEQXSLA5CNFSM4IDJNII2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4JLSKI#issuecomment-521320745, or mute the thread https://github.com/notifications/unsubscribe-auth/AD6FZCGLI33HDGQGUGS6METQEQXSLANCNFSM4IDJNIIQ .

rasoolsomji commented 5 years ago

@amosbastian I am honestly not sure... are you getting that error message on any particular test?

amosbastian commented 5 years ago

Basically all the tests fail for me locally, same for Travis CI. Only thing I can think of that could be causing it is the operating system. I'm on Ubuntu, what about you?

rasoolsomji commented 5 years ago

Yup same, Ubuntu 16.04, and Python 3.6.8. I'm baffled.

What happens if you manually try to make the changes I made in utils.py to a new local branch? That's the only file I significantly changed

amosbastian commented 5 years ago

Same thing happens. It's really confusing and everything I find about it on StackOverflow etc. doesn't solve it. Not sure what we should do next, haha.

rasoolsomji commented 5 years ago

Haha me neither, I guess it's just one of those things. Maybe let's leave it a while and come back to it, it might have sorted itself out