cleanup to get closer to CRAN submission #75
Conversation
|
d0d98f4 is more substantive. It makes |
|
Note that |
There was a problem hiding this comment.
This took me less time than I thought. Overall, it looks good. There are one or two changes I'm less familiar with, but trust what you've done. Did have one suggested change to the scrape_statcast_savant.Date function to make sure users can pull games regardless of category (i.e. regular season, post season, etc.).
| playerid_var <- ifelse(player_type == "pitcher", | ||
| "pitchers_lookup%5B%5D", "batters_lookup%5B%5D") | ||
|
|
||
| vars <- tibble::tribble( |
There was a problem hiding this comment.
Added to the tibble that is created. This allows for any games to be included, including post season and spring training. Some fields may be unnecessary, but added just in case.
vars <- tibble::tribble(
~var, ~value,
"all", "true",
"hfPT", "",
"hfAB", "",
"hfBBT", "",
"hfPR", "",
"hfZ", "",
"stadium", "",
"hfBBL", "",
"hfNewZones", "",
"hfGT", "R%7CPO%7CS%7C&hfC",
"hfSea", paste0(lubridate::year(start_date), "%7C"),
"hfSit", "",
"hfOuts", "",
"opponent", "",
"pitcher_throws", "",
"batter_stands", "",
"hfSA", "",
"player_type", player_type,
"game_date_gt", as.character(start_date),
"game_date_lt", as.character(end_date),
"hfInfield", "",
"team", "",
"position", "",
"hfOutfield", "",
"hfRO", "",
"home_road", "",
"pitchers_lookup%5B%5D", ifelse(is.null(592789), "", 592789),
"hfFlag", "",
"hfPull", "",
"metric_1", "",
"hfInn", "",
"min_pitches", 0,
"min_results", 0,
"group_by", "name",
"sort_col", "pitches",
"player_event_sort", "h_launch_speed",
"sort_order", "desc",
"min_abs", 0,
"type", "details"
) %>%
dplyr::mutate_(pairs = ~paste(var, "=", value, sep = ""))
There was a problem hiding this comment.
Sure, but what is this about?
"pitchers_lookup%5B%5D", ifelse(is.null(592789), "", 592789),
There was a problem hiding this comment.
I guess ultimately it would be cool if users could pass values for all of these values through the .... Or maybe the function could take a params list?
There was a problem hiding this comment.
I literally kept telling myself "make sure to copy the right one, make sure to copy the right one".
Re: all the values, yeah that was my thought. Figured it was worth putting them all in now.
Nothing really substantive here -- just bringing things in line with CRAN policy/practice