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

Helpers #32

Open
wants to merge 36 commits into
base: master
Choose a base branch
from
Open

Helpers #32

wants to merge 36 commits into from

Conversation

tcash21
Copy link
Contributor

@tcash21 tcash21 commented May 20, 2016

@Btibert3 This is so awesome. Any tests I should run in particular? I'll probably run a bunch of basic scripts I have for each sport and then vote to merge.

Changed the version to major 1, as it seems like we haven’t really had
issues with the package beyond the need for helpers
Added show_data helper, built docs
introduce draft of intermediary helpers, modify hockey teams and games
# Conflicts:
#	README.md
all four sports now have a helper base.  Needs to be tested too.
First tests passed, removed temp functions not yet developed
Added higher level helpers for common game log and team game log use
cases.  First pass testing passed.
Simply bug on teams helpers, started vignette
Another bug existed on the injuries helpers, and finished the first
pass of the vignette.
Allow vignette files to seen by git. Still need to figure out how to
build for all users, not just locally.
I think this works.  Needed to tweak gitignore
properly handle team and game parameters if left blank.  Add Walk
parameter.
Modified clean_sideload_divisions to pull in division name data for
both the team and their opponent
typo in test if to filter logs based on an individual team.  Allows
query for all teams, or individual team if specified
@Btibert3
Copy link
Contributor

@tcash21 I would run through your code base or some of the recipes that you run often. That said, I noticed that on my older work machine I was hitting an error on some of the sport-specific helpers which I hadn't encountered before. Any and all things that you can think of. It's pretty solid (methinks), but as we keep stacking on top of the core functions, the dependencies are growing. Throw whatever you can at it, and I will look at the sport helpers tonight and start to think about a better test suite.

For future reference, this is the error I had thrown on the basic teams call

 > mlb <- hockey_teams()
[1] "Making initial API request"
Joining by: "league_id"
Error: No common variables. Please specify `by` param.

Bug with join between divisions and conferences
@Btibert3
Copy link
Contributor

There was a simple bug in the join on the *_teams helper when we join divisions and conference side-loaded data.

@tcash21
Copy link
Contributor Author

tcash21 commented May 23, 2016

Any reason to not just have the user use select in dplyr instead of our own custom ss_keep_cols?

@Btibert3
Copy link
Contributor

The ability to control for invalid columns specified by the user.

On Sun, May 22, 2016 at 8:21 PM, tcash21 [email protected] wrote:

Any reason to not just have the user use select in dplyr instead of our
own custom ss_keep_cols?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#32 (comment)

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.

4 participants