Closed richiehu17 closed 3 years ago
Thanks for the PR @richiehu17. I'll aim to review it within the next week.
Hey @richiehu17, I remember the SOS feature from last year. I was reading our discussions in #12 and it looked like the feature was too resource-intensive to deploy to the main site so we agreed on putting it in the more-features
branch. Before I review the code, I was wondering if you've done anything to improve the performance of this feature so that it doesn't time out and/or crash the website?
If the performance still causes the website to crash, I can change the target branch of this PR to be more-features
and continue reviewing it. However, it will not be deployed to the main site 😞 . As always, thanks for your contribution!
This current PR uses the ESPN fantasy API so the entire SOS call just makes 1 API call to get all the scoreboard information.
@wcrasta Just to be more specific, the SOS/Weekly performance endpoints don't use the soup scraping anymore, just one ESPN API call for scoreboard info.
Awesome! Things are a bit hectic for me so I might not get to reviewing the whole thing for a few days, but I'll definitely post here if I have any questions as I look into the code.
Apologies for the late review. I haven't forgotten about this. Hope to get to it this weekend.
Hi @richiehu17, since Fantasy Playoffs have started (or are starting) in many leagues, I wanted to deploy this soon so that your great work does not go unnoticed. I think I'll deploy it to the website, but review the code later since I'm still quite busy.
I'm reviewing the Season Strength of Schedule for leagueId=633975. The page says: Higher average opponent rank means a harder schedule.
. If I understood correctly, doesn't a numerically lower average opponent rank mean a harder schedule?
Same with the Overall Performance. Shouldn't it be that numerically lower average means better performance?
@wcrasta oops yeah you're right; that's from the previous way it was written. I'll make a quick change
@wcrasta I just updated the descriptions and rebased to include the version bumps
Just deployed your code to the website. It's blazing fast, great job! Although my league doesn't use ESPN anymore, I'm excited that the website users will have these interesting new tools. Whenever I get around to looking at how you implemented it, I'll try and see if I can use the same logic for my Yahoo tools website. I haven't seen any other website compute strength of schedule, so I love it!
Once again, I apologize for the length of time it took for me to deploy your code. Myself and everyone who uses the website are very appreciative of all your contributions! Going forward, I'll try to be better about reviewing submissions promptly or at the very least, providing a definitive timeline for when I might get to it. I'll let you know if I receive any feedback about these new tools.
@wcrasta I think the new SOS/weekly performance will throw exceptions somewhere in the code when there is a bye week in the playoffs. I'll make some changes sometime this weekend or next weekend to see if I can change it to just regular season games.
Got it, no worries. It's still a nice feature to have so I merged it in. If you get a chance to fix it, that's great. No rush though, since the fantasy season is likely coming to a close for many players.
SOS and weekly performance use the ESPN scoreboard API call. There needs to be a 'mMatchupScore' param to get 'matchupPeriodId' keys. I didn't change the core matchups/rankings because I'm a little unsure how well/consistently it works.
I changed SOS and performance to use weekly rankings, don't know why I didn't do that in the first place. They use average rank in the calculations.
I also have 0 clue what is going to happen to the methods when fantasy playoffs start. I'd like to add a check to see if playoffs have started and if I can cap the week count to the regular season, but I'm planning on just waiting and seeing what the API calls look like when the playoffs start.