Skip to content

fix: event_num typo, missing DB write in load_nba_team_box, uninitialized games in scoreboards#186

Open
jacobpstein wants to merge 1 commit intosportsdataverse:mainfrom
jacobpstein:fix/pbp-typo-loader-db-scoreboard-init
Open

fix: event_num typo, missing DB write in load_nba_team_box, uninitialized games in scoreboards#186
jacobpstein wants to merge 1 commit intosportsdataverse:mainfrom
jacobpstein:fix/pbp-typo-loader-db-scoreboard-init

Conversation

@jacobpstein
Copy link
Copy Markdown

@jacobpstein jacobpstein commented Apr 20, 2026

Summary

I'm a big fan of hoopR and use it extensively for my Wizards coverage at wizardspoints.substack.com. While working with the package I ran into a few bugs I've been meaning to address — fixing them here.

Changes

  • R/nba_stats_pbp.R (lines 154–155): Typo even_numevent_num in the .players_on_court() fallback branch (V2 path). The correct column name event_num is used at lines 99–100 in the same function; this branch missed the update. Triggers when a player is subbed on and off at the exact same clock time in the same period.

  • R/load_nba.R: load_nba_team_box() accepts dbConnection and tablename parameters and sets in_db <- TRUE, but never applies the flag. The if (in_db) DBI::dbWriteTable(...) block present in load_nba_pbp() and load_nba_player_box() was missing entirely. Callers passing a DB connection silently received a tibble instead of writing to the database.

  • R/nba_stats_scoreboard.R: nba_scoreboardv3(), nba_schedule(), and nba_todays_scoreboard() assign games inside their tryCatch expr block but call return(games) outside it. If the API errors, games is never defined and the function crashes with "object 'games' not found" instead of returning gracefully. Added games <- list() before each tryCatch, matching the pattern used by the deprecated nba_scoreboardv2() and all other df_list-returning functions in the file.

Test plan

  • nba_pbp(game_id = "0022200021", version = "v2", on_court = TRUE) — confirm no column-not-found error
  • load_nba_team_box(seasons = 2023, dbConnection = con, tablename = "team_box") with a DBI connection — confirm data written to DB
  • Call nba_scoreboardv3(), nba_schedule(), nba_todays_scoreboard() with an invalid date — confirm graceful list() return instead of crash
  • devtools::check() — 0 new errors or warnings

Summary by CodeRabbit

  • New Features

    • Team box score data can now be persisted to a configured database for easier data management
  • Bug Fixes

    • Fixed substitution event tracking in play-by-play data to ensure accurate player rotation information
    • Enhanced robustness of scoreboard query functions during error conditions

…ninitialized games in scoreboards

- R/nba_stats_pbp.R: typo even_num -> event_num in .players_on_court()
  fallback branch (lines 154-155). Correct column event_num is used at
  lines 99-100 in the same function; this branch missed the update.
  Triggers when a player is subbed on and off at the exact same clock
  time in the same period (V2 path, nba_pbp(..., version = "v2")).

- R/load_nba.R: load_nba_team_box() accepted dbConnection/tablename
  parameters and set in_db <- TRUE but never applied the flag. Added the
  if (in_db) DBI::dbWriteTable(...) block matching the pattern already
  present in load_nba_pbp() and load_nba_player_box().

- R/nba_stats_scoreboard.R: nba_schedule(), nba_scoreboardv3(), and
  nba_todays_scoreboard() assigned games inside tryCatch expr but called
  return(games) outside it. On API error, games was undefined and the
  function crashed instead of returning gracefully. Added games <- list()
  before each tryCatch, matching the df_list <- list() pattern used by
  all other functions in the file.
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 20, 2026

Someone is attempting to deploy a commit to the sportsdataverse Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 20, 2026

📝 Walkthrough

Walkthrough

These changes modify three R functions: adding conditional database writing to load_nba_team_box(), correcting a column reference in .players_on_court(), and explicitly initializing a variable in three scoreboard functions before error handling blocks.

Changes

Cohort / File(s) Summary
Database Persistence
R/load_nba.R
Modified load_nba_team_box() to conditionally write loaded data to database when in_db is true and dbConnection/tablename are provided. Returns NULL on database write; otherwise returns in-memory classed tibble as before.
Play-by-Play Data Processing
R/nba_stats_pbp.R
Fixed column reference in .players_on_court() substitution handling: changed from pbp_data_period$even_num to pbp_data_period$event_num for proper event ordering comparison.
Scoreboard API Error Handling
R/nba_stats_scoreboard.R
Explicitly initialized games <- list() before tryCatch() blocks in nba_schedule(), nba_scoreboardv3(), and nba_todays_scoreboard() to ensure defined return value on error paths.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A bunny hops through data with care,
Writes to databases, no column repair!
Event_num fixed, errors now caught,
Clean initialization—just what we sought! 🎲

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes all three main bug fixes in the changeset: the event_num typo in nba_stats_pbp.R, the missing DB write in load_nba_team_box, and uninitialized games in scoreboards functions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
R/nba_stats_scoreboard.R (1)

88-88: LGTM — prevents object 'games' not found on API error.

Initializing games <- list() before each tryCatch ensures the error path returns a valid (empty) value instead of crashing. One minor note: on the success path these functions return a tibble while the error path now returns list() — the return type is inconsistent across branches. If callers downstream expect a data frame shape, consider games <- dplyr::tibble() instead. Non-blocking; the pattern matches existing sibling functions (df_list <- list()).

Also applies to: 684-684, 837-837

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@R/nba_stats_scoreboard.R` at line 88, You initialized games <- list() to
avoid "object 'games' not found" on API error, but that makes the error path
return a list while the success path returns a tibble; change the initializer to
dplyr::tibble() (or tibble::tibble()) so both success and error branches return
the same data-frame-like type, and apply the same change to the other
occurrences of games <- list() in the file (the similar blocks around the other
tryCatch usages).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@R/nba_stats_scoreboard.R`:
- Line 88: You initialized games <- list() to avoid "object 'games' not found"
on API error, but that makes the error path return a list while the success path
returns a tibble; change the initializer to dplyr::tibble() (or
tibble::tibble()) so both success and error branches return the same
data-frame-like type, and apply the same change to the other occurrences of
games <- list() in the file (the similar blocks around the other tryCatch
usages).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: db470937-c62e-491a-ac48-99b02c2a5f8c

📥 Commits

Reviewing files that changed from the base of the PR and between d425ec5 and 3153114.

📒 Files selected for processing (3)
  • R/load_nba.R
  • R/nba_stats_pbp.R
  • R/nba_stats_scoreboard.R

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.

1 participant