Updates to Prep for CRAN

saiemgilani commented 3 years ago

1. Some GitHub workflow updates

Create bug_report.md
Create feature_request.md
Create pkgdown.yaml
Create R-CMD-check.yaml
Update .Rbuildignore
Delete .travis.yml (moving to R-CMD-Check github workflow since Travis-CI now has limited support for continuous integration for open source projects and GitHub Actions has stepped in as a drop-in replacement. (when these workflows run, they show up in the repository Actions tab.) GitHub Actions are highly useful, versatile, can be used for a number of useful things other than just package checks and documentation website building.

2. An attempt at updating some documented datasets and documentation

Create data-raw/create_bref_abbreviation.R … This is a small function to adjust some things for team names in differing eras. it's sorta working, could probably take a little more elbow grease
Update teams_lu_table.rda … upated from data-raw/create_bref_abbreviation.R
Slight updates to documentation scrape_savant_leaderboards() had a typo in one of the arguments leading to some warnings.
Previously undocumented teams_lu_table and stats_api_live_empty_df

3. Do it for the CRAN

Another behavior that may be unfamiliar to you is that I had to remove any assigning of datasets to the global environement throughout the package. That's basically not allowed from what I understand.
rlang's .data$ requirement and adding explicit function calls like … …dplyr::
Fixing these .[length(.)-1] things as well as the previous commit message things
add import of rlang and data documentation …
Improving examples, upgrading syntax of underlying code to modern dplyr (> 0.7.0) styles.

4. Tests (work in progress)

adding tests
- I went straight down the R/ file list and I think I am basically done with the tests I am going to write.
- So far there are 79 test checks in the package These are the ones not getting tests since they appear to be applied on a dataframe (without the pipeline written in the examples, I didn't want to just guess.)
  - fip_plus.R
  - ggspraychart.R
  - linear_weights_savant.R
  - process_statcast_payload.R
  - run_expectancy_code.R
  - run_expectancy_table.R
  - statline_from_statcast.R
  - viz_gb_on_period.R
  - woba_plus.R

Boils down to vizzes and model prep code. If it's written outside the pipeline and there isn't a convenience function, it'd take me more time than I'd care to in order to figure out what goes where.

A couple minor things to fix the tests so that they would pass. The main thing is I added a supressWarnings to the read_csv in any of the player lookup functions... and I added supressWarnings to basically any function that did a conversion to numeric that results in the creation of NA's.

5. Substantive code change to fix small issues or simplify

Update batter_game_logs_fg.R … fix grepl to actually extract the % before trying to rename it (it causes it to null out otherwise)
Update get_ncaa_baseball_roster.R … substantive change to get same end result.

6. Package Documentation and Website details

Update Readme and Description … adding myself as a contributor
Create extra.css … Just for the style
A thing worthy of discussion is the pkgdown website. I don't understand what was going on with the gh-pages branch, but I basically deleted it to get my version of the pkgdown website to work. If that is where you were keeping your other site up and running this shouldn't be an issue but I will need to change the pkgdown workflow.

BillPetti commented 3 years ago

Still working through, but to your question about gh-pages--that's where I used to host a site about the package. Now it is built through pkgdown and is built in the docs folder on the master branch.

BillPetti commented 3 years ago

I'm gonna merge this into my dev branch and then let's see how it works. My guess is we've got some other work to do as I've made some bug fixes to the master since these were pushed, etc.

BillPetti commented 3 years ago

@saiemgilani

So I ran a check on the dev branch and here's what I got:

── R CMD check results ──────────────────────────────────── baseballr 0.9.9 ────
Duration: 2m 51.8s

> checking tests ...
  See below...

> checking dependencies in R code ... NOTE
  Missing or unexported object: ‘rvest::html_elements’

> checking R code for possible problems ... NOTE
  get_ncaa_lineups: no visible binding for global variable ‘playerName’
  Undefined global functions or variables:
    playerName

── Test failures ───────────────────────────────────────────────── testthat ────

> library(testthat)
> library(baseballr)
> 
> test_check("baseballr")
[1] "Be patient, this may take a few seconds..."
[1] "Data courtesy of the Chadwick Bureau Register (https://github.com/chadwickbureau/register)"
[1] "Be patient, this may take a few seconds..."
[1] "Data courtesy of the Chadwick Bureau Register (https://github.com/chadwickbureau/register)"
[1] "Be patient, this may take a few seconds..."
[1] "Data courtesy of the Chadwick Bureau Register (https://github.com/chadwickbureau/register)"
[1] "Be patient, this may take a few seconds..."
[1] "Data courtesy of the Chadwick Bureau Register (https://github.com/chadwickbureau/register)"
══ Failed tests ════════════════════════════════════════════════════════════════
── Error (test-batter_game_logs_fg.R:11:3): Fangraphs Batter Game Logs ─────────
Error: 'html_elements' is not an exported object from 'namespace:rvest'
Backtrace:
    █
 1. ├─baseballr::batter_game_logs_fg(playerid = 6184, year = 2017) test-batter_game_logs_fg.R:11:2
 2. │ ├─`%>%`(...)
 3. │ └─xml2::read_html(url) %>% rvest::html_elements("table")
 4. ├─base::as.data.frame(.)
 5. ├─rvest::html_table(., fill = TRUE)
 6. └─rvest::html_elements
 7.   └─base::getExportedValue(pkg, name)
── Error (test-get_ncaa_baseball_roster.R:11:3): NCAA Get Roster ───────────────
Error: Table has inconsistent number of columns. Do you want fill = TRUE?
Backtrace:
    █
 1. ├─baseballr::get_ncaa_baseball_roster(teamid = 104, team_year = 2019) test-get_ncaa_baseball_roster.R:11:2
 2. │ └─(payload %>% rvest::html_nodes("table"))[[1]] %>% rvest::html_table()
 3. ├─rvest::html_table(.)
 4. └─rvest:::html_table.xml_node(.)
── Error (test-milb_batter_game_logs_fg.R:16:3): FanGraphs MiLB Batter Game Logs ──
Error: 'html_elements' is not an exported object from 'namespace:rvest'
Backtrace:
    █
 1. ├─baseballr::milb_batter_game_logs_fg(playerid = "sa917940", year = 2018) test-milb_batter_game_logs_fg.R:16:2
 2. │ ├─`%>%`(...)
 3. │ └─xml2::read_html(url_adv) %>% rvest::html_elements("table#SeasonStats1_dgSeason2_ctl00")
 4. ├─base::as.data.frame(.)
 5. ├─rvest::html_table(.)
 6. └─rvest::html_elements
 7.   └─base::getExportedValue(pkg, name)

saiemgilani commented 3 years ago

@saiemgilani

So I ran a check on the dev branch and here's what I got:

── R CMD check results ──────────────────────────────────── baseballr 0.9.9 ────
Duration: 2m 51.8s

> checking tests ...
  See below...

> checking dependencies in R code ... NOTE
  Missing or unexported object: ‘rvest::html_elements’

> checking R code for possible problems ... NOTE
  get_ncaa_lineups: no visible binding for global variable ‘playerName’
  Undefined global functions or variables:
    playerName

── Test failures ───────────────────────────────────────────────── testthat ────

> library(testthat)
> library(baseballr)
> 
> test_check("baseballr")
[1] "Be patient, this may take a few seconds..."
[1] "Data courtesy of the Chadwick Bureau Register (https://github.com/chadwickbureau/register)"
[1] "Be patient, this may take a few seconds..."
[1] "Data courtesy of the Chadwick Bureau Register (https://github.com/chadwickbureau/register)"
[1] "Be patient, this may take a few seconds..."
[1] "Data courtesy of the Chadwick Bureau Register (https://github.com/chadwickbureau/register)"
[1] "Be patient, this may take a few seconds..."
[1] "Data courtesy of the Chadwick Bureau Register (https://github.com/chadwickbureau/register)"
══ Failed tests ════════════════════════════════════════════════════════════════
── Error (test-batter_game_logs_fg.R:11:3): Fangraphs Batter Game Logs ─────────
Error: 'html_elements' is not an exported object from 'namespace:rvest'
Backtrace:
    █
 1. ├─baseballr::batter_game_logs_fg(playerid = 6184, year = 2017) test-batter_game_logs_fg.R:11:2
 2. │ ├─`%>%`(...)
 3. │ └─xml2::read_html(url) %>% rvest::html_elements("table")
 4. ├─base::as.data.frame(.)
 5. ├─rvest::html_table(., fill = TRUE)
 6. └─rvest::html_elements
 7.   └─base::getExportedValue(pkg, name)
── Error (test-get_ncaa_baseball_roster.R:11:3): NCAA Get Roster ───────────────
Error: Table has inconsistent number of columns. Do you want fill = TRUE?
Backtrace:
    █
 1. ├─baseballr::get_ncaa_baseball_roster(teamid = 104, team_year = 2019) test-get_ncaa_baseball_roster.R:11:2
 2. │ └─(payload %>% rvest::html_nodes("table"))[[1]] %>% rvest::html_table()
 3. ├─rvest::html_table(.)
 4. └─rvest:::html_table.xml_node(.)
── Error (test-milb_batter_game_logs_fg.R:16:3): FanGraphs MiLB Batter Game Logs ──
Error: 'html_elements' is not an exported object from 'namespace:rvest'
Backtrace:
    █
 1. ├─baseballr::milb_batter_game_logs_fg(playerid = "sa917940", year = 2018) test-milb_batter_game_logs_fg.R:16:2
 2. │ ├─`%>%`(...)
 3. │ └─xml2::read_html(url_adv) %>% rvest::html_elements("table#SeasonStats1_dgSeason2_ctl00")
 4. ├─base::as.data.frame(.)
 5. ├─rvest::html_table(.)
 6. └─rvest::html_elements
 7.   └─base::getExportedValue(pkg, name)

You're not using rvest >= 1.0.0 is why, I'm guessing. The ' fill' parameter was deprecated in html_table (it just does a better job, it's not necessary anymore), and html_elements is essentially a drop-in replacement for html_node.

I have been trying to update the pull to match the other branches as they've happened, so I'm hoping it is minimal.

For example, I commited a CRAN-passable version of the statcast payload and fixed the milb funcs. b71e21c b3f5f96

BillPetti / baseballr