-
Notifications
You must be signed in to change notification settings - Fork 62
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
Fix duplicate player hrefs in FBref functions #332
Conversation
@@ -48,7 +48,7 @@ | |||
return(tibble::tibble()) | |||
} | |||
renamed_table <- .rename_fb_cols(tables[[3]]) | |||
renamed_table[renamed_table$Rk != "Rk", ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
drive by fix... this should have been assigned back to renamed_table
😅
R/get_advanced_match_stats.R
Outdated
res <- dplyr::mutate( | ||
df, | ||
"Player_Href" = players[df$Player], | ||
"Player_Href" = player_hrefs, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this assumes the number of player links will be equal to the number of rows in df
. this is probably a fine assumption
Codecov Report
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. @@ Coverage Diff @@
## main #332 +/- ##
==========================================
- Coverage 63.18% 63.09% -0.10%
==========================================
Files 44 44
Lines 3583 3582 -1
==========================================
- Hits 2264 2260 -4
- Misses 1319 1322 +3
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
It was possible for FBref functions that returned
Player_Href
(added with #328) to return "bad" links when there are different players with the same exact name. This is fixed by simply joining the hrefs back to the rest of the dataframe rather than doing a lookup by name. The new approach may encounter an error if a player does not have an FBref page for some reason, although I've never actually seen that before.Fixes #327.