Better handling for NaN in betting lines #96
Conversation
Signed-off-by: Akshay Easwaran <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Apply Sweep Rules to your PR?
|
Signed-off-by: Akshay Easwaran <[email protected]>
Signed-off-by: Akshay Easwaran <[email protected]>
WalkthroughThe update to the Changes
Assessment against linked issues
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (1)
- R/cfbd_betting.R (2 hunks)
Additional comments: 3
R/cfbd_betting.R (3)
49-49: The addition of
stringr::str_replace_allto handle NaN values is a good approach to ensure that the JSON parsing does not fail due to unexpected 'NaN' strings.140-150: The use of
stringr::str_replace_allto replace 'NaN' with 'null' before parsing the JSON response is correct. However, ensure that this replacement does not affect any other unintended parts of the response data.Verification successful
The verification process has confirmed that the
stringr::str_replace_allfunction is used to replace 'NaN' with 'null' only once in thecfbd_betting.Rfile. There are no other instances of 'NaN' that could be unintentionally affected by this operation.* 146-149: The mutation using `dplyr::mutate` and `dplyr::case_when` to handle 'null' values in the 'overUnder' column is correctly implemented. It ensures that 'null' strings are converted to `NA_character_`, which is the appropriate NA type for character columns in R.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that 'NaN' values only occur where they represent missing numeric data and not in other contexts. rg --type R --glob 'cfbd_betting.R' 'NaN'Length of output: 65
Script:
#!/bin/bash # Adjusting the script to search for 'NaN' in 'cfbd_betting.R' without specifying the file type. rg --glob 'cfbd_betting.R' 'NaN'Length of output: 101
Signed-off-by: Akshay Easwaran <[email protected]>
Signed-off-by: Akshay Easwaran <[email protected]>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- R/cfbd_betting.R (2 hunks)
Additional comments: 3
R/cfbd_betting.R (3)
- 140-140: The use of
stringr::str_replace_all("NaN", 'null')effectively addresses the issue of handling NaN values in the dataset. This change ensures that the JSON parsing does not fail due to NaN values, which are not valid JSON.- 145-158: The conditional mutations for
overUnder,spread, andformattedSpreadusingdplyr::case_whenare correctly implemented. However, ensure that.data$overUnder,.data$spread, and.data$formattedSpreadare correctly referenced on the right-hand side inside thedplyr::case_when, as previously noted bysaiemgilani. This ensures that the correct columns are being mutated based on the conditions provided.- 160-160: The conditional logic for handling the
line_providerparameter and the creation of a default data frame when no data is available or when filtering byline_providerare correctly implemented. The suggestion to refactor the repeated code for creating an empty data frame into a function to avoid duplication and improve maintainability is a good practice. This refactoring makes the code cleaner and more maintainable.
Closes #93.
Summary by CodeRabbit