Skip to content

refactor: unify league update/summary entry points and fix params cross-refs (#42) - #59

Merged
pbulsink merged 2 commits into
masterfrom
fix-42-api-cleanup
Oct 7, 2026
Merged

pbulsink merged 2 commits into
masterfrom
fix-42-api-cleanup

Conversation

@pbulsink

@pbulsink pbulsink commented Oct 6, 2026

Copy link
Copy Markdown
Owner

Summary

As a user of HockeyModel, in order to keep the public API small and consistent, I would like a single, league-agnostic entry point for updating and summarising both the NHL and PWHL, rather than separate league-specific exports that shadow the generic names.

This PR makes updateModel() and dailySummary() the single public entry points for both leagues. The former league-specific exports (updateDC(), updatePWHLDC(), updatePWHLModel(), dailyPWHLSummary()) are now internal, and the params argument across the NHL prediction/simulation/social-media functions now points at the correct source of the params list.

Behavior

1. League-specific exports demoted to internal

Before: Four public functions duplicated the behaviour of the generic updateModel() / dailySummary() while leaking league-specific names into the API:

Public export Role
updateDC() Fit NHL Dixon-Coles params
updatePWHLDC() Fit PWHL Dixon-Coles params
updatePWHLModel() Update PWHL schedule + scores + params
dailyPWHLSummary() PWHL daily summary / social post

After: updateModel() and dailySummary() are the only public entry points. The league-specific functions are renamed to internal helpers and no longer exported:

Internal helper Replaces
.update_dc_nhl() updateDC()
.update_dc_pwhl() updatePWHLDC()
.update_model_pwhl() updatePWHLModel()
.daily_summary_pwhl() dailyPWHLSummary()

NAMESPACE drops the four exports; devtools::document() removes the four exported .Rd pages and generates internal ones (dot-*.Rd) marked @keywords internal.

2. params cross-references fixed

Before: ~40 exported NHL functions documented their params argument with See [updateDC] — a link to a function that no longer exists once #42 is applied, and which is PWHL-agnostic in name but NHL-specific in reality.

After: Those links point at updateModel(), the function that actually produces the NHL params list. The PWHL data-object docs (pwhl_m, pwhl_rho, pwhl_beta, pwhl_eta, pwhl_k) link to .update_dc_pwhl(), their real populator.

Tests

Updated every test that referenced the renamed functions to mock/call the internal names:

  • test-frontend-core.R — updateModel() dispatch tests now mock .update_model_pwhl().
  • test-frontend-daily-summary.R / test-frontend-plots.R — mocks now use .daily_summary_pwhl().
  • test-pwhl-daily-summary.R — test descriptions and the called function renamed to .daily_summary_pwhl().
  • test-pwhl-model-core.R — updatePWHLDC / updatePWHLModel test blocks renamed to .update_dc_pwhl / .update_model_pwhl; mock of updatePWHLDC now mocks .update_dc_pwhl.
  • test-nhl-dixon-coles-workflow.R / test-nhl-dixon-coles-predict.R — updateDC(...) calls renamed to .update_dc_nhl(...).
  • helper-pwhl.R — comment updated.

Full suite: 803 pass, 0 fail, 0 warn, 0 skip.

Notes

  • devtools::check(error_on = "warning"): 0 errors, 0 warnings, 1 NOTE. The NOTE (no visible binding for global variable 'i' in loopless_sim / simulateSeasonParallel) is pre-existing on master (introduced by the function(i) closure in PR perf: optimize season simulation loops (#45, #49, #51) #57) and unrelated to this change — this PR only touches a doc cross-reference in nhl-league-simulation.R, no code touching i.
  • The 5 previously-untracked internal .Rd files (dcPredictVectorized.Rd, dcProbArray.Rd, sim_batch.Rd, sim_odds_results.Rd, simulation_chunks.Rd) are now committed; they were generated by devtools::document() in prior PRs but never staged, and are required for Rd cross-references ... OK.
  • NEWS.md updated with a single user-facing bullet under the dev heading.
  • No new public API; the package surface strictly shrinks by 4 exports.

References

…ss-refs (#42)

Makes updateModel() and dailySummary() the single public entry points for both leagues. The former league-specific exports updateDC(), updatePWHLDC(), updatePWHLModel(), and dailyPWHLSummary() are now internal (.update_dc_nhl(), .update_dc_pwhl(), .update_model_pwhl(), .daily_summary_pwhl()), and the params argument across the NHL prediction/simulation/social-media functions now points at the correct params source.

- Closes #42
Apply the R convention that package-internal helpers carry a leading dot:
71 previously unexported functions (DCweights, prob_matrix, dcPredictVectorized,
build_past_predictions, parseCores, buildTeamColours, getConferences,
single_series_solver, etc.) are now .DCweights, .prob_matrix,
.dcPredictVectorized, etc. across R/, tests/testthat/, and vignettes/.

devtools::document() regenerates man/*.Rd (old .Rd pages deleted, dot-*.Rd
pages created) and NAMESPACE (no change — these were never exported).
NAMESPACE and DESCRIPTION are unchanged; only internal names shift.

devtools::test: 803 pass, 0 fail.
devtools::check: 0 errors, 0 warnings, 1 pre-existing NOTE (i binding,
from PR #57, unrelated to this change).
@pbulsink
pbulsink merged commit ed393d7 into master Oct 7, 2026
6 checks passed
@pbulsink
pbulsink deleted the fix-42-api-cleanup branch October 14, 2026 11:33
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.

API Cleanup

1 participant