pittcsc / PittAPI

An API to easily get data from the University of Pittsburgh
https://pittapi.pittcsc.org
GNU General Public License v2.0
108 stars 33 forks source link

Add sports.py #115

Closed rohit-ganguly closed 4 years ago

rohit-ganguly commented 5 years ago

Created a new script for accessing Pitt Sports information via ESPN's hidden endpoints for their sports API (a completely free and legal option).

Currently, there is support for:

With potential to add Baseball and Women's basketball.

There are methods for:

RitwikGupta commented 5 years ago

Ignore the failed build, but can you write a mocked test for this? See the existent tests for reference. I'll start a review on this this week.

RitwikGupta commented 5 years ago

@timparenti Thoughts on including this? I'm torn on the "pureness" of it. On one hand, it's Pitt data, and that was the goal of this API when I made it. On the other hand, it's not Pitt data from Pitt APIs. I'm leaning towards the former and going for inclusion here.

rohit-ganguly commented 5 years ago

I can totally write more tests, I'll take a look and get to that ASAP. If you'd prefer I can go for a more Pitt-oriented route and scrape from Pitt's sites purely to fit your second point a bit more.

Such a method was actually the plan for the other sports programs at Pitt (volleyball, tennis, soccer, etc.) that don't have such API support from ESPN and would require scraping of data. I too don't want to pull everything from something that isn't ours.

Again, I can definitely take that approach with the existing sports (Women's basketball, Men's basketball, Football) if you'd like to uphold that value, but I do think the direct access to ESPN's endpoints makes this information easier to access, faster to retrieve, and has the perk of being continually updated without any hassle on our end.

Let me know what you guys think @timparenti @RitwikGupta

timparenti commented 5 years ago

@RitwikGupta On the one hand, the general format of the ESPN API is probably significantly more stable than scraping from the Pitt Athletics website, which changes quite frequently. On the other hand, this makes use of hidden endpoints which are liable to have their public access shut off without warning.

I think that, as long as the feature is labelled experimental here, the key will be ensuring that it degrades gracefully when the endpoints aren't available (e.g., raising a proper/custom exception). With those caveats, I think this sort of thing is probably fine to include, as I suspect it would be difficult to achieve a more resilient approach, at least not without paying for access to proprietary sports APIs.

rohit-ganguly commented 4 years ago

Update:

I've followed the advice given and made changes/additions.

  1. The next game info methods for both of the currently implemented sports now return a dictionary with all the information, following @azharichenko's suggestion.
  2. The API's are called only inside the methods, following @timparenti's suggestion.
  3. I've written very basic tests for the methods in this file, following @RitwikGupta's suggestion.
  4. I've updated all mentions of basketball methods to include "Men's" before them - good catch @timparenti.

Let me know if there are any other changes you see fit.

RitwikGupta commented 4 years ago

@azharichenko @timparenti Once your reviews are in, I'll merge