uberfastman / yfpy

Python API wrapper for the Yahoo Fantasy Sports public API (supports NFL, NHL, MLB, and NBA).
https://pypi.org/project/yfpy/
GNU General Public License v3.0
163 stars 44 forks source link

Typing inconsistencies #30

Closed AsteroidOrangeJuice closed 1 year ago

AsteroidOrangeJuice commented 1 year ago

I am trying to use query.get_league_teams, and am expecting a List[Team], but am instead getting a List[dict].

Is there a better way to do this so I get the types I am expecting?

As an example, this outputs "dict", but should output "Team", based on the Mypy types.

query = YahooFantasySportsQuery(Path("/credentials/yfpy_auth"), league_id=league_id, browser_callback=False)
league_teams = query.get_league_teams()
logger.info(type(league_teams[0]))
mille5a9 commented 1 year ago

I think the team object should be the value associated with the 'team' key in the dict. Try league_teams[0]['team'].

AsteroidOrangeJuice commented 1 year ago

Hm,

league_teams = query.get_league_teams()
logger.info(type(league_teams[0]["team"]))

does seem to contain the team object:

<class 'yfpy.models.Team'>

But, that does not match the return type of List[Team], it seems like List[Dict[str: Team]] is more accurate, but that seems unintended.

uberfastman commented 1 year ago

@mille5a9 thanks for chiming in with the data structure info. @AsteroidOrangeJuice it seems you've caught a typing documentation error on my part... the representation of List[Dict[str: Team]] is actually the currently intended structure, although I have to admit that looking at it now I wish it was just List[Team] as that is much cleaner and more user-friendly.

The reason it is set up with the the single key dictionary wrapping the actual YFPY object is so that the data can be serialized and deserialized from the retrieved JSON (or saved json). Since the data coming from Yahoo is pretty messy, the logic I built in to parse, clean, and in some cases reorganize that retrieved JSON data uses that "team" key (in this case) to detect that the object should be deserialized into a YFPY Team object. Without that key, it doesn't currently have another way of detecting what the top level object needs to be.

I think it would be worthwhile to try and do the above differently, but the primary challenge with trying to use things like fields in the JSON to "smartly" detect what the object is is that when data is absent, Yahoo just omits those fields, so some objects don't include all the possible fields they can have, so not all instances of a "team" object data will contain all the same set of fields, leading us to the challenge of trying to figure out a better way to detect what class the data should be deserialized into.

So in summation, the type documentation is incorrect (thanks for catching that, I'll try and get it fixed shortly), but the current data structures are as intended in order to support "intelligent" typecasting/deserialization into usable YFPY objects.

uberfastman commented 1 year ago

@AsteroidOrangeJuice YFPY 8.0.8 now includes updated documentation to properly reflect the expected data structures. I still think an overall cleaner solution would be the best approach, but at least in the meantime the docs accurately describe what to expect.