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

Add Advanced Stats Functionality (#96) #101

Open
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

hirememorey
Copy link

This PR introduces advanced stats functionality to the NHL API. It addresses issue #96 by adding new modules and examples.

  • advanced_stats.py: Implements the advanced stats retrieval methods.
  • advanced_parsers.py: Contains parsing logic to handle the advanced stats response data.
  • examples/: Provides example scripts to demonstrate the usage of these new endpoints.
  • cookies.py: Updates cookie handling to support additional API endpoints.

Testing Instructions:

  1. Clone this branch and install any necessary dependencies.
  2. Run the example scripts in the examples/ folder to see the advanced stats in action.
  3. Please refer to the inline comments and documentation in the code for further details.

I welcome any feedback or suggestions for further refinement.

@coreyjs coreyjs self-assigned this Feb 14, 2025
@coreyjs
Copy link
Owner

coreyjs commented Feb 14, 2025

Thanks for this! I'll be away until 2/23, so I don't think I can give this a full review before then, but I will try.

@coreyjs coreyjs added the enhancement New feature or request label Feb 14, 2025
@coreyjs coreyjs removed their assignment Feb 14, 2025
@coreyjs coreyjs self-requested a review February 14, 2025 14:45
@coreyjs
Copy link
Owner

coreyjs commented Feb 25, 2025

So this is introducing design patterns that are separate from the core API library, almost as if they are separate utilities. Do you think there is a way to incorporate this into the current design structure, for something like:

client.advanced_stats.goalie_stats()

I can see the difficulty in running asynchronous web sockets to load this, but having it incorporated into the current library design would be beneficial for end users.

Depending on difficulty, could do some kind of utilities options, where it can be installed as an optional package include.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request inreview
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants