search

keberwein / mlbgameday

Multi-core processing of 'Gameday' data from Major League Baseball Advanced Media. Additional tools to parallelize large data sets and write them to a database.
Other
41 stars 8 forks source link

Remove menu and just warn on get_payload. Added team DF to gd_innings_all so team names can be added to data #14

Closed atroiano closed 5 years ago

atroiano commented 5 years ago

Removed the menu, when using this with large machines, having to wait for a menu to pop up is not ideal.

keberwein commented 5 years ago

OK, I've reviewed the pull.

I like the idea to remove the warning about large data sets. When I wrote this package, the average computer had 4Gb to 8Gb of memory, so that made sense at the time, but probably not anymore.

I'm not totally sold on the idea of adding a team column to innings_all data set at this point. Reasons are:

So... Since I can't accept just part of a pull request, could you either A) Un-do the the last 11 commits of this pull, and only keep the first 2. Or B) Just close this PR all together and submit a new one with only the first two changes.

I'll add you as a contributor in the description and keep you up-to-date for the development branch that deals with the Statcast API. I'm hoping to have that work finished the the beginning of next season.

Thanks so much, glad you're enjoying the package!

atroiano commented 5 years ago

Ok cool, I am 100% fine with that too. Pulling the linescore data as you suggested doesn't add materially more time to the scrapes. I'll close the PR and open a new one.

I'm also open to helping port to the Statcast API so holler if you need some help!

On Sat, Apr 13, 2019 at 9:57 AM Kris Eberwein notifications@github.com wrote:

OK, I've reviewed the pull.

I like the idea to remove the warning about large data sets. When I wrote this package, the average computer had 4Gb to 8Gb of memory, so that made sense at the time, but probably not anymore.

I'm not totally sold on the idea of adding a team column to innings_all data set at this point. Reasons are:

-

We have a lot of people that do daily pulls and load them into a relational database to keep history. Adding an extra column would break such functionality. It's a good idea, but not something I want to do in the middle of the season.

The next major change to this package will be to move away from the Gameday API and re-port the entire thing to the Statcast API. Now that the Gameday API is deprecated, I don't want to throw too much more effort into it.

So... Since I can't accept just part of a pull request, could you either A) Un-do the the last 11 commits of this pull, and only keep the first

  1. Or B) Just close this PR all together and submit a new one with only the first two changes.

I'll add you as a contributor in the description and keep you up-to-date for the development branch that deals with the Statcast API. I'm hoping to have that work finished the the beginning of next season.

Thanks so much, glad you're enjoying the package!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/keberwein/mlbgameday/pull/14#issuecomment-482811518, or mute the thread https://github.com/notifications/unsubscribe-auth/APNWWZv_lRVheszpi4GPbJVvfw_zfaSLks5vgeJBgaJpZM4ctbW7 .

atroiano commented 5 years ago

Closing will open a new one with only requested changes.