All the same functionality in package form #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…able issue was that the parsing of Weeks --> Week numbers was broken; I took the lazy-ish approach of sidestepping the problem entirely, and fixing it after-the-fact in my new week_name_stopgap file. Specifically, the issue was with trying to split a dictionary in line 69 (commented out in this commit).
… produces a file game_meta_data_ready_to_merge.csv which can be left-joined to a set of plays via season/week/hometeam/awayteam
|
Wow this all really has a mind of its own, sorry for the phantom close and reopen of the pull request! Hopefully you're not being bombarded with notifications from all this. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hey greerre,
Awesome code here! My name is Dennis, and I'm (among other things) involved in sports analytics by way of co-running the Puntalytics twitter account. The Puntalytics team is really interested in weather data, so I was super excited to find your scripts. You mentioned in the README that you'd love to make this all a package at some point, and I happen to personally be in the process of teaching myself python package development. To that end, I've made the changes you see in my branch.
If you'd prefer, rather than merge my changes directly into your own repo (which seems like a large, potentially risky step!) you could add a new branch to your repo, and I'll resubmit the pull request to that. I'm new to the exact choreography of forks and merges, so I'm open to other suggestions as well.
Just about everything you wrote worked without a hitch on my computer. There were two minor exceptions: first, the parsing of Week to Week_number in line 69 (original line number, new line number 82) of pfr_game_link_scraper wasn't working for me, and I couldn't quite determine the root of the error. I Instead decided to just drop that line, and then handle the week --> week_number conversion separately in week_name_stopgap. If making that problematic line work again is easy, and I'm just missing something, it would be straightforward to correct the pipeline to make that work.
The other weird thing was that I got a KeyError when merging dataframes because of the "away_won_toss" header. Looking back as I type this, I think maybe it should have been "home_won_toss", but I just commented out the line and moved on. Should be an easy fix.
Converting the main body of each script into a function went smoothly. For pfr_meta_data_format, I reorganized the file such that global variables go first, then helper functions, then the overall function. For the other files, pretty much everything is in the same place it started. The biggest changes were making sure that the input and output file paths were taken in as variables.
Hope you enjoy! Let me know if you have any questions/concerns about the changes I mentioned here or about the init.py and .gitignore files that construct the package on the back end.