Closed aemabe closed 1 month ago
1 files 8 suites 12s :stopwatch: 53 tests 53 :white_check_mark: 0 :zzz: 0 :x: 384 runs 384 :white_check_mark: 0 :zzz: 0 :x:
Results for commit a28b190d.
:recycle: This comment has been updated with latest results.
Test Suite | $Status$ | Time on main |
$±Time$ | $±Tests$ | $±Skipped$ | $±Failures$ | $±Errors$ |
---|---|---|---|---|---|---|---|
correlogram | 👶 | $+2.44$ | $+33$ | $0$ | $0$ | $0$ |
Results for commit e137f346da3c7180bfd9780fd0c0bb9a97969781
♻️ This comment has been updated with latest results.
@aemabe Thank you for creating the PR. Some comments for your consideration: 1) In the function, variables are formatted as binary, continuous, or ordinal based on their values. Then different formulas are used for correlation calculation depending on the type of the variables. I suggest adding some documentation on this so that the users are aware how their data are being processed by the function and what formulas are used for correlation calculation to ensure what the function does aligns with their expectations. This will also alert users to format their input data appropriately before loading them into this function. For example, they should ensure that ordinal variables in their dataset are of character type in order to be recognized as ordinal variables by the function. 2) Related to comment 1), I am wondering whether it makes more sense that we require a variable to be of factor type rather than character type in order to recognize it as an ordinal variable, because an ordinal variable could be numerical, e.g., an ordinal variable may take values of 1, 2, 3, 4. 3) Suggest adding some tests to ensure that the function works as expected (e.g., correct formulas are used) for different types of variables.
Review the pkgdown webpage for the PR here
fantastic work Abigail and Chen!
Code Coverage Summary
Diff against main
Results for commit: a28b190da0687e20449dffd37657028b513eb41c
Minimum allowed coverage is
80%
:recycle: This comment has been updated with latest results