-
Notifications
You must be signed in to change notification settings - Fork 60
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
Has anyone noticed that when you use fotmob_get_matches_by_date, a different output occurs nearly 50% of the time? #244
Comments
can you please provide some code with a specific example? |
It's hard to explain -- just it sometimes works and sometimes doesn't particularly when run through with a bunch of other code or as part of a loop. Once it works it seems to work again and again though. |
i'm sorry mate, but we can't help if there is no example. here's a simple example to show that the function does respect inputs (although i'm not thoroughly checking the integrity of the results) library(worldfootballR)
packageVersion('worldfootballR')
#> [1] '0.6.2.6000'
matches_sat <- fotmob_get_matches_by_date('2023-01-14')
dim(matches_sat)
#> [1] 315 33
## should be different from Saturday's results
matches_sun <- fotmob_get_matches_by_date('2023-01-15')
dim(matches_sun)
#> [1] 228 33
## should be different from the prior two
matches_mon1 <- fotmob_get_matches_by_date('2023-01-16')
dim(matches_mon1)
#> [1] 50 33
## should be the same as above
matches_mon2 <- fotmob_get_matches_by_date(as.Date('2023-01-16'))
dim(matches_mon2)
#> [1] 50 33
## this shouldn't work
matches_mon3 <- fotmob_get_matches_by_date('01/16/2023')
dim(matches_mon3)
#> [1] 0 0 |
This happens to me fairly regularly. At this moment (22 Jan 1623 hrs CET) when I have it run for a number of dates, it will not pull the current date. ` match_id_df <- fotmob_get_matches_by_date(future_dates) |
For the record, three minutes later, it works. This happens to me regularly however. I have a script that runs automatically overnight and roughly one day out of seven the fotmob_get_matches_by_date() formula fails to pull games for the current date (or maybe all dates, I can't be sure). |
i'm not sure how to help. i'm not totally sure this is a one thing i suppose we could do in the package is to fail quickly and loudly, instead of silently. In this case, we might set f <- function(url) {
resp <- safely_from_json(url)$result
res <- resp$leagues %>%
janitor::clean_names() %>%
tibble::as_tibble()
if(nrow(res) == 0) {
stop(sprintf("Could not find data for %s.", url))
return(res)
}
res %>%
dplyr::rename(match = .data[["matches"]]) %>%
tidyr::unnest(.data[["match"]], names_sep = "_") %>%
dplyr::rename(home = .data[["match_home"]], away = .data[["match_away"]]) %>%
tidyr::unnest(c(.data[["home"]], .data[["away"]], .data[["match_status"]]), names_sep = "_") %>%
dplyr::select(-tidyselect::vars_select_helpers$where(is.list)) %>%
janitor::clean_names()
}
fp <- purrr::possibly(f, otherwise = tibble::tibble())
fp(url) this would give you an error message for one day, but wouldn't actually fail because we could also just not use wdyt @JaseZiv |
Yeah this is a tough one... I also don't think this is a I like your first possibly remedy @tonyelhabr. If possible, I think it would be neat if we told the user explicitly which dates were problems, yet continued getting everything else, rather than a hard fail through the process. Thoughts? |
Yeah I do that myself with my daily scripts, sending myself a message if there are zero results for a given date. Makes sense to me to just highlight for the user when there might be an issue, and to specify the "failed" dates if possible.
On Sun, Jan 22, 2023 at 20:46, Jason ***@***.***> wrote:
Yeah this is a tough one... I also don't think this is a worldfootballR issue, but is definitely one we can help with in the library.
I like your first possibly remedy @tonyelhabr. If possible, I think it would be neat if we told the user explicitly which dates were problems, yet continued getting everything else, rather than a hard fail through the process.
Thoughts?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you commented.Message ID: ***@***.***>
|
i've got this PR open to address this. should be merged soon |
That's also what I had as a solution -- appreciate the feedback. Glad I wasn't crazy and the only experiencing it. |
PR merged - thanks heaps @tonyelhabr! Install the most recent dev version ( Thanks |
There has to be some cacheing issue or something but the function is completely ignoring my inputs and commonly ignoring the first date given.
The text was updated successfully, but these errors were encountered: