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

Implements tidy.Kendall + adds info in DESCRIP, NAMESPACE, etc.. #343

Merged
merged 1 commit into from Jun 11, 2018

Conversation

cimentadaj
Copy link
Contributor

Following #285, I've added tidy.Kendall which creates a tidy method for all Kendall class objects. These come from the Kendall package and only have 3 functions: Kendall, MannKendall and SeasonalMannKendall. They all return the same information so the tidy method works for the three functions. The number of rows and columns are always the same and there are not optional arguments on the three functions.

Here's a brief example of what the tidy.Kendall will return

library(Kendall)
library(broom)

### For Kendall ####
A <- c(2.5, 2.5, 2.5, 2.5, 5, 6.5, 6.5, 10, 10, 10, 10, 10, 14, 14, 14, 16, 
  17)
B <- c(1, 1, 1, 1, 2, 1, 1, 2, 1, 1, 1, 1, 1, 1, 2, 2, 2)
f_res <- Kendall(A, B)

tidy(f_res)
#> # A tibble: 1 x 5
#>     tau p.value kendall_score denominator var_kendall_score
#>   <dbl>   <dbl>         <dbl>       <dbl>             <dbl>
#> 1 0.408  0.0754            34        83.4              345.

### For MannKendall ####
s_res <- MannKendall(B)

tidy(s_res)
#> # A tibble: 1 x 5
#>     tau p.value kendall_score denominator var_kendall_score
#>   <dbl>   <dbl>         <dbl>       <dbl>             <dbl>
#> 1 0.354   0.102            32        90.3               360

### For SeasonalMannKendall ####
t_res <- SeasonalMannKendall(ts(A))

tidy(t_res)
#> # A tibble: 1 x 5
#>     tau     p.value kendall_score denominator var_kendall_score
#>   <dbl>       <dbl>         <dbl>       <dbl>             <dbl>
#> 1 0.924 0.000000935           116        126.              559.

Everything I've done so far:

  • devtools::check() is not passing but there are no warnings from tidy.Kendall.
  • goodpractice::gp() passes for tidy.Kendall
  • devtools::spell_check() only returns the word kendall which is a false positive.
  • I've reviewed the tidyverse documentation guidelines and fixed everything accordingly in the docs of tidy.Kendall
  • I've updated NEWS.md to include tidy.Kendall.
  • I've added tests using check_tidy in test-kendall.R
  • I've added the Kendall package as Suggests
  • I've added myself as ctb

@cimentadaj cimentadaj mentioned this pull request Jun 10, 2018
@rbloehm
Copy link

rbloehm commented Jun 10, 2018

Hi cimentada, there seems to be an issue with the line endings (should probably be LF) or the encoding (should be UTF-8) of the files in your commit.
See for example DESCRIPTION, NAMESPACE, NEWS.md. You should only see changes in the 1-2 lines you touched, but somehow every single line changed.
broom.Rproj actually hasn't changed at all, but is still shown as changed in every single row.

@cimentadaj
Copy link
Contributor Author

@rbloehm You're completely right. I have this behaviour ever since I started using Git and Bash in Windows 10. I'm not sure how to fix it at this point with out reverting everything back and re implementing the changes.

@rbloehm
Copy link

rbloehm commented Jun 10, 2018

Sorry about that, I hope you will find some guidance here:
https://help.github.com/articles/dealing-with-line-endings/#platform-windows

Especially make sure that
git config --global core.autocrlf true
is set globally on your Windows machine.

@alexpghayes
Copy link
Collaborator

This looks really good! Thanks for taking the time to make a PR!

I have a suspicion that at least one of the columns in the output should be called statistic for consistency, but won't be able to check until the work week. Besides that, let's figure out how to get the line endings fixed and then get this thing merged!

@cimentadaj cimentadaj force-pushed the tidy.Kendall branch 5 times, most recently from da83477 to 834be8b Compare June 11, 2018 07:56
@cimentadaj
Copy link
Contributor Author

cimentadaj commented Jun 11, 2018

@alexpghayes @rbloehm I fixed the problem with line endings and changed the column name of tau to statistic for consistency. Let me know if that's alright!

@alexpghayes alexpghayes merged commit ccc6878 into tidymodels:master Jun 11, 2018
@alexpghayes
Copy link
Collaborator

Looks great, thank you!

@cimentadaj cimentadaj deleted the tidy.Kendall branch June 11, 2018 19:29
@rbloehm
Copy link

rbloehm commented Jun 11, 2018

@alexpghayes: Was still committed and merged into master with CRLF in the new files. Run this in the latest master of tidyverse/broom:
git grep -I --files-with-matches --perl-regexp '\r' HEAD
image
Still a nice PR though, thanks @cimentadaj

@alexpghayes
Copy link
Collaborator

Thanks!

@github-actions
Copy link

This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants