Remove dependency on usethis and progressr #134
Conversation
…functions + other cleanups. Use rlang::check_installed() over some customized code for suggested package handling.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Apply Sweep Rules to your PR?
|
|
You a real one for this. For the record, I wrap the function names in the documents with the triple backticks so that {pkgdown} will definitely auto-link the functions appropriately. I don't just think that's an appropriate generally, it's more of a means to an end. If something has changed with how that works, lmk. If not, I'd appreciate a quick reversal on that piece of editing |
|
It works with single back ticks now :) See https://dplyr.tidyverse.org/news and https://github.com/tidyverse/dplyr/blob/main/NEWS.md?plain=1 |
Oh bet, good tip, I suppose I will merge it when I get a chance to review tonight or so. Appreciate your help! |
Signed-off-by: olivroy <52606734+olivroy@users.noreply.github.com>
Signed-off-by: olivroy <52606734+olivroy@users.noreply.github.com>
olivroy
changed the title
Signed-off-by: olivroy <52606734+olivroy@users.noreply.github.com>
WalkthroughThe recent changes to the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Signed-off-by: olivroy <52606734+olivroy@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Out of diff range and nitpick comments (5)
NEWS.md (5)
16-30: Consider varying the sentence structure to enhance readability.The repeated use of "function added" at the beginning of each item can be monotonous. Consider rephrasing some entries or combining them to improve the flow of the text.
Line range hint
6-338: Standardize the list item markers.The list items throughout the document inconsistently use dashes (
-) instead of asterisks (*). For markdown consistency, consider using asterisks (*) for all list items.
Line range hint
12-306: Correct the heading levels.The document uses heading levels inconsistently, sometimes skipping levels. This can disrupt the logical structure of the document. Ensure that heading levels increment by one level at a time.
Line range hint
5-343: Ensure blank lines around headings.Some headings in the document do not have the required blank lines above and below them. This can affect readability and the proper rendering of the markdown. Please add a blank line above and below each heading.
Line range hint
6-341: Ensure blank lines around lists.Some lists in the document do not have blank lines before and after them. This is necessary for proper markdown rendering. Please add blank lines around all lists.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
logo.pngis excluded by!**/*.png
Files selected for processing (19)
- .Rbuildignore (1 hunks)
- DESCRIPTION (2 hunks)
- NEWS.md (7 hunks)
- R/espn_mbb_data.R (2 hunks)
- R/espn_nba_data.R (3 hunks)
- R/load_mbb.R (13 hunks)
- R/load_nba.R (13 hunks)
- R/nba_stats_pbp.R (3 hunks)
- R/nba_stats_player.R (1 hunks)
- R/nba_stats_video.R (1 hunks)
- R/utils.R (7 hunks)
- R/utils_nba_stats.R (2 hunks)
- README.Rmd (3 hunks)
- README.md (3 hunks)
- data-raw/pull_team_links.R (1 hunks)
- man/csv_from_url.Rd (2 hunks)
- man/hoopR-package.Rd (1 hunks)
- tests/testthat/test-kp_arenas.R (1 hunks)
- vignettes/getting-started-hoopR.Rmd (6 hunks)
Files skipped from review due to trivial changes (8)
- .Rbuildignore
- R/nba_stats_player.R
- R/nba_stats_video.R
- README.Rmd
- data-raw/pull_team_links.R
- man/csv_from_url.Rd
- man/hoopR-package.Rd
- vignettes/getting-started-hoopR.Rmd
Additional Context Used
LanguageTool (27)
NEWS.md (24)
Near line 8: Unpaired symbol: ‘]’ seems to be missing
Context: ...layers for each play event, courtesy of [@shufinskiy](https://github.com/shufins...
Near line 16: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...tion added. -nba_todays_scoreboard()function added. ### **NBA Boxscore V3 (and V3-s...
Near line 22: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ion added. -nba_boxscoreadvancedv3()function added. -nba_boxscoremiscv3()functio...
Near line 23: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...unction added. -nba_boxscoremiscv3()function added. -nba_boxscorescoringv3()func...
Near line 24: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...tion added. -nba_boxscorescoringv3()function added. -nba_boxscoreusagev3()functi...
Near line 25: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nction added. -nba_boxscoreusagev3()function added. -nba_boxscorefourfactorsv3()...
Near line 26: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... added. -nba_boxscorefourfactorsv3()function added. -nba_boxscoreplayertrackv3()...
Near line 27: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... added. -nba_boxscoreplayertrackv3()function added. -nba_boxscorematchupsv3()fun...
Near line 28: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ion added. -nba_boxscorematchupsv3()function added. -nba_boxscorehustlev2()funct...
Near line 29: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ction added. -nba_boxscorehustlev2()function added. -nba_boxscoredefensivev2()fu...
Near line 30: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...on added. -nba_boxscoredefensivev2()function added. ### **Other NBA Stats API funct...
Near line 36: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... added. -nba_franchiseleaderswrank()function added. -nba_videodetailsasset()func...
Near line 37: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...tion added. -nba_videodetailsasset()function added. - `nba_infographicfanduelplayer(...
Near line 38: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ded. -nba_infographicfanduelplayer()function added. -nba_teams()function added. ...
Near line 39: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ayer()function added. -nba_teams()` function added. ### Other Functions Added ...
Near line 57: You’ve repeated a verb. Did you mean to only write one of them?
Context: .... - Regular minor maintenance onkp_*functions -espn_mbb_game_rosters()function added. -espn_nba_game_rosters()func...
Near line 70: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nction added. -espn_nba_team_stats()function added. # hoopR 1.7.0 - `nba_pbps()...
Near line 76: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...github.com/billyfryer). -nbagl_pbp()function added, courtesy of [@billyfryer](https...
Near line 77: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ....com/billyfryer). -nbagl_standings()function added, courtesy of [@billyfryer](https...
Near line 78: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...com/billyfryer). -nba_gamerotation()function added. # hoopR 1.6.0 - `nba_player...
Near line 259: This phrase is duplicated. You should probably use “function added” only once.
Context: ...e.org/reference/load_mbb_team_box.html) function added -hoopR::load_nba_team_box()function added ### Add player box score loaders -...
Near line 263: This phrase is duplicated. You should probably use “function added” only once.
Context: ...org/reference/load_mbb_player_box.html) function added -hoopR::load_nba_player_box()function added # hoopR 1.0.5 ### **Standings func...
Near line 289: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...to be more identity information related only - All tests were updated ### **Loading...
Near line 315: This phrase is duplicated. You should probably use “dependency added” only once.
Context: ...ded -tibbleversion 3.0.0 or greater dependency added -furrrdependency added -futuredependency added ### **Test...README.md (3)
Near line 56: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...full play-by-play seasons (2002-2024) ~ 1-2 minutes** ``` r tictoc::tic() nba_pbp ...
Near line 68: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...full play-by-play seasons (2006-2024) ~ 2-3 minutes** ``` r tictoc::tic() mbb_pbp ...
Near line 105: Consider using a more formal and expressive alternative to ‘awesome’.
Context: ... /> ## Our Contributors (they’re awesome) - Jason Lee (@theFirmAISports) ...
Additional comments not posted (19)
DESCRIPTION (2)
35-35: Ensure all links are functional after changing to HTTPS.Verification successful
The verification process aimed to ensure that the updated HTTPS link in the DESCRIPTION file is functional. The script executed a
curlcommand to check the HTTP status code of the URLhttps://hoopr.sportsdataverse.org. The output confirmed that the URL responded with a status code of200, which indicates that the server successfully processed the request and that the link is functional.Given this result, we can conclude that the HTTPS link is operational as expected.
HTTPS Link Verification Complete
The HTTPS link provided in the DESCRIPTION file is functional:
https://hoopr.sportsdataverse.orgresponded with HTTP 200 OK.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all HTTPS links are functional. # Test: Check the status code of each HTTPS URL. Expect: HTTP 200 OK. curl -o /dev/null -s -w "%{http_code}\n" https://hoopr.sportsdataverse.orgLength of output: 80
41-41: Updated dependency onclito version3.1.0aligns with the PR's objectives to replaceprogressrfunctionalities.R/utils_nba_stats.R (1)
46-46: Introduction ofrequest_with_proxyfunction enhances flexibility in handling HTTP requests with specific proxy and header requirements.R/utils.R (2)
37-37: Updatedloginfunction signature improves clarity and default handling of environment variables.
245-245: Introduction ofhoop_todoandhoop_infofunctions for better message handling aligns with the PR's objectives to enhance user and developer interactions.R/load_nba.R (4)
102-106: Replacedprogressrwithclifor progress tracking. Ensure that the new progress tracking integrates well with existing workflows.
216-220: Progress tracking updated to usecli. Verify that this change integrates smoothly with the rest of the application.
325-329: Updated to useclifor progress tracking. Consistency with other data loading functions is maintained.
448-452: Progress tracking method updated tocli. Ensure that this change is well integrated into the system.R/load_mbb.R (4)
98-102: Replacedprogressrwithclifor progress tracking. Ensure that the new progress tracking integrates well with existing workflows.
195-199: Progress tracking updated to usecli. Verify that this change integrates smoothly with the rest of the application.
308-312: Updated to useclifor progress tracking. Consistency with other data loading functions is maintained.
441-445: Progress tracking method updated tocli. Ensure that this change is well integrated into the system.R/nba_stats_pbp.R (4)
344-344: Replace the deprecatedprogressr::progressorwithcli::cli_progress_alongfor progress handling.
344-344: Ensure that thecli::cli_progress_alongfunction is used correctly for progress tracking.Verification successful
The verification process has confirmed that the
cli::cli_progress_alongfunction is indeed used within thenba_pbpfunction in theR/nba_stats_pbp.Rfile, as indicated by the search results. The function is used with the parametersgame_idsandname = "Loading", which suggests that it is being used to track the progress of loading game IDs.Given this information, it appears that the
cli::cli_progress_alongfunction is being utilized correctly for progress tracking in the context of thenba_pbpfunction. This aligns with the expected usage of the function for providing feedback on the progress of data loading operations.
- The usage of
cli::cli_progress_alonginnba_stats_pbp.Ris correct and aligns with the intended purpose of progress tracking.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct usage of `cli::cli_progress_along` in the function `nba_pbp`. # Test: Search for the function usage. Expect: Correct parameters and usage context. rg --type r $'cli::cli_progress_along'Length of output: 664
477-477: Validate the presence of game IDs before proceeding with data retrieval.
477-477: Check the correct handling of game IDs and ensure robust error messages are provided.Verification successful
The verification process has revealed that the
nba_live_pbpfunction in thenba_stats_pbp.Rfile does indeed implement error handling mechanisms. Specifically, the function usestryCatchfor exception handling andstopto generate error messages when necessary conditions are not met, such as missing game IDs. This confirms that the function has robust error handling as required by the review comment.
- The use of
tryCatchis evident in multiple files across the repository, indicating a consistent approach to error handling.- The
stopfunction is used innba_stats_pbp.Rto enforce the presence of game IDs, which aligns with the review comment's concern about handling game IDs correctly.Based on this information, the error handling in the
nba_live_pbpfunction is verified to be robust and user-friendly, addressing the concerns raised in the review comment.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the error handling in `nba_live_pbp`. # Test: Search for error handling patterns. Expect: Robust and user-friendly error messages. rg --type r $'error = function(e)'Length of output: 35
Script:
#!/bin/bash # Description: Further investigate error handling in `nba_live_pbp`. # Test: Search for common R error handling patterns and inspect the function directly. # Search for tryCatch which is a common error handling method in R rg --type r 'tryCatch' # Additionally, search for stop and warning functions which are used for generating errors and warnings in R rg --type r 'stop\(' rg --type r 'warning\('Length of output: 16607
R/espn_nba_data.R (1)
Line range hint
1370-1391: Namespace adjustment fordplyr::whereis correctly implemented.R/espn_mbb_data.R (1)
Line range hint
1426-1447: Namespace adjustments fordplyr::where()are correctly implemented to avoid conflicts.Verification successful
The verification process has successfully confirmed that all instances of
dplyr::selectin the R files across the codebase are using the namespace explicitly, as intended. This ensures that there are no conflicts with other packages that might also have aselectfunction, adhering to best practices in R programming for package development.Summary of Verification:
- All occurrences of the
selectfunction from thedplyrpackage are prefixed withdplyr::, which is the correct usage to avoid namespace conflicts.- The script checked across various R files in the repository, including those within the
data-raw,R, and other directories, confirming consistent usage.This verification confirms that the namespace adjustments for
dplyr::selectare correctly implemented throughout the project.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `dplyr::select` match the new namespace usage. # Test: Search for the function usage. Expect: Only occurrences of the new namespace usage. rg --type R $'dplyr::select'Length of output: 53
Script:
#!/bin/bash # Description: Verify all function calls to `dplyr::select` match the new namespace usage. # Test: Search for the function usage in all R files. Expect: Only occurrences of the new namespace usage. rg 'dplyr::select' --glob '*.R'Length of output: 10249
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- vignettes/getting-started-hoopR.Rmd (6 hunks)
Files skipped from review as they are similar to previous changes (1)
- vignettes/getting-started-hoopR.Rmd
Use cli for messages. https://cli.r-lib.org/articles/usethis-ui.html
I also made some cleanups here and there.
I also removed progressr dependency in favour of cli which has support for progress bars.
I made use of rlang::check_installed() when necessary.
I removed unused Suggests.
Since
where()is now exported by dplyr, I namespaced it instead of adding it to globalvariables.Cheers
Summary by CodeRabbit
New Features
hoop_todoandhoop_infofor enhanced message display with timestamps.Enhancements
csv_from_urlfunction description to clarify its role as a wrapper arounddata.table::fread().Documentation
progressr::with_progresswrapper.Bug Fixes
loginfunction for consistency.Refactor
progressr,usethis, and others to streamline package requirements.Tests