Skip to content
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

Remove fotmob functions #311

Merged
merged 6 commits into from
Aug 12, 2023
Merged

Remove fotmob functions #311

merged 6 commits into from
Aug 12, 2023

Conversation

tonyelhabr
Copy link
Collaborator

@tonyelhabr tonyelhabr commented Aug 8, 2023

Fotmob updated their terms of service stating that "the use of automatic services (robots, spiders, indexing etc.) as well as other methods for systematic or regular use is not permitted." While "robots, spiders, etc." doesn't necessarily imply data scraping is off limits, the fotmob CEO has written us something akin to a "cease and desist" notice insisting that we remove our fotmob functions. We're not in a position to counter, so we've decided to comply.

@tonyelhabr tonyelhabr changed the title Rm fotmob Remove fotmob functions Aug 8, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #311 (e23da2e) into main (60a9fc8) will decrease coverage by 4.98%.
The diff coverage is n/a.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main     #311      +/-   ##
==========================================
- Coverage   67.72%   62.74%   -4.98%     
==========================================
  Files          49       43       -6     
  Lines        4065     3358     -707     
==========================================
- Hits         2753     2107     -646     
+ Misses       1312     1251      -61     
Files Changed Coverage Δ
R/get_match_lineups.R 44.60% <ø> (ø)
R/internals.R 89.65% <ø> (+0.95%) ⬆️
R/tm_team_staff_history.R 98.21% <ø> (ø)

... and 1 file with indirect coverage changes

@@ -411,16 +411,3 @@
session <- rvest::session(url = page_url, ua)
xml2::read_html(session)
}

#' @importFrom httr GET content
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

these functions were only used by fotmob functions

@@ -300,12 +300,12 @@ test_that("fb_team_player_stats() works", {
Sys.sleep(3)
test_that("fb_team_match_log_stats() works", {
testthat::skip_on_cran()
liv_keeper_log <- fb_team_match_log_stats(team_url= "https://fbref.com/en/squads/822bd0ba/Liverpool-Stats", stat_type= 'keeper', time_pause = 4)
liv_keeper_log <- fb_team_match_log_stats(team_url= "https://fbref.com/en/squads/822bd0ba/2022-2023/Liverpool-Stats", stat_type= 'keeper', time_pause = 4)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

drive by fix: this was failing, i think because it was looking for 2023/2024 stats

@tonyelhabr tonyelhabr requested a review from JaseZiv August 8, 2023 12:05
Copy link
Owner

@JaseZiv JaseZiv left a comment

Choose a reason for hiding this comment

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

Thanks for all your work on this @tonyelhabr

@JaseZiv JaseZiv merged commit 14ea127 into main Aug 12, 2023
@JaseZiv JaseZiv deleted the rm-fotmob branch August 12, 2023 03:08
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.

3 participants