Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include Total Points in player_stats return #52

Merged

Conversation

DMcP89
Copy link
Collaborator

@DMcP89 DMcP89 commented Sep 15, 2024

I have updated the player_stats command to leverage the League resource from the Yahoo API instead of the Player resource. This allows us to return the total points and all the stats details. There are a few other benefits IMO:

  • Makes the library more predictable as the league class is now using the league resource instead of an edge case where it uses the player resource
  • We are only gaining data here the league /stats endpoint returns the same values as the players /stats endpoint
  • Allows us to simplify the mock for player_stats

@duncanduncan
Copy link

I have updated the player_stats command to leverage the League resource from the Yahoo API instead of the Player resource. This allows us to return the total points and all the stats details. There are a few other benefits IMO:

  • Makes the library more predictable as the league class is now using the league resource instead of an edge case where it uses the player resource
  • We are only gaining data here the league /stats endpoint returns the same values as the players /stats endpoint
  • Allows us to simplify the mock for player_stats

Thanks for your work on this.
Does this allow for checking against league scoring rules to product a players total fantasy points?

@DMcP89
Copy link
Collaborator Author

DMcP89 commented Sep 16, 2024

@duncanduncan yes the total points is based off the scoring of the league. The point calculation is done by the Yahoo API not the library so it will always match what's listed in the Yahoo app/site

@DMcP89
Copy link
Collaborator Author

DMcP89 commented Sep 17, 2024

@spilchen Can this be merged?

Copy link
Owner

@spilchen spilchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks for contributing again.

@spilchen spilchen merged commit 6b26e6b into spilchen:master Sep 18, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants