JaseZiv / worldfootballR

A wrapper for extracting world football (soccer) data from FBref, Transfermark, Understat
https://jaseziv.github.io/worldfootballR/
433 stars 59 forks source link

Has anyone noticed that when you use fotmob_get_matches_by_date, a different output occurs nearly 50% of the time? #244

Closed mbrownsword closed 1 year ago

mbrownsword commented 1 year ago

There has to be some cacheing issue or something but the function is completely ignoring my inputs and commonly ignoring the first date given.

tonyelhabr commented 1 year ago

can you please provide some code with a specific example?

mbrownsword commented 1 year ago

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.

tonyelhabr commented 1 year ago

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
cg86x commented 1 year ago

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.

` future_dates <- seq(Sys.Date(), Sys.Date()+21, by = "days")

match_id_df <- fotmob_get_matches_by_date(future_dates) ` This returns no matches for Jan 22, 2023.

cg86x commented 1 year ago

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).

tonyelhabr commented 1 year ago

i'm not sure how to help. i'm not totally sure this is a {worldfootballR} issue. it could very well be that the fotmob API is down intermittently for a few seconds, but then comes back online.

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 quiet = FALSE here and add a message about not finding data.

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 possibly() serves like a tryCatch() that just moves on to the next input.

we could also just not use purrr::possibly() (or, equivalently, tryCatch()). this would result in an error that stops the call to fotmob_get_matches_by_date from returning any data whatsoever.

wdyt @JaseZiv

JaseZiv commented 1 year ago

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?

cg86x commented 1 year ago

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: @.***>

tonyelhabr commented 1 year ago

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

mbrownsword commented 1 year ago

That's also what I had as a solution -- appreciate the feedback. Glad I wasn't crazy and the only experiencing it.

JaseZiv commented 1 year ago

PR merged - thanks heaps @tonyelhabr!

Install the most recent dev version (0.6.2.8000) and should be good to go.

Thanks