Add fct_reordern() (Fix #16)

[billdenney]

Fix #16

@hadley, I saw what you meant in the #16 discussion about affecting the value and not just the levels. That is addressed here. Also, I removed the .desc argument. Since it was both simpler and potentially friendlier to the user to leave the decreasing argument from base::order() exposed (it doesn't have the dplyr dependency to sort decreasing), I left that. And, I made the default method "radix" so that it would work with a vector of values for decreasing (something that would annoy me whenever I forgot that I needed to set the method, and it seemed like a minor wart to add an argument directly).

Anyway, let me know what you think and if I'm missing something else important.

[billdenney]

The integration check is only in R 3.2, and it is that when running one of the new examples, the following error is generated:

> A <- c(3, 3, 2, 1)
> B <- c("A", "B", "C", "D")
> fct_reordern(.f = c("A", "B", "C", "D"), A, B)
Error in base::order(..., method = method) : argument lengths differ

Looking at historical documentation (https://www.rdocumentation.org/packages/base/versions/3.2.0/topics/order), order() in R 3.2.0 didn't support the method argument. Is the preference to add code to protect R 3.2 or something else?

[hadley]

You can remove the tests for decreasing = TRUE because you are just testing the behaviour of an existing function.

[billdenney]

Removed except for one to confirm that interaction between the two works as decreasing sorting is an important behavior.

[hadley]

That adds a dependency on dplyr — instead you'll need to implement a local version of desc() inside the test, e.g. desc <- function(x) -xtfrm(x)

[billdenney]

For my education, I thought that since the dplyr call is in a test and dplyr is already in the DESCRIPTION Suggests field, I thought that this was OK. (Similarly, since I have it in the examples in the help, I thought that the same applied there.) I'm happy to implement that way, but my hope was that this would be simpler for the user (via help) and future maintainer (via tests) to use.

Whichever way you confirm, I'll update to that.

[billdenney]

I believe that I have addressed all comments in the update that is about to be committed.

[billdenney]

@hadley, I think that I have addressed everything.

[billdenney]

FYI, I have re-updated to current main and fixed all CI issues (mainly the updated issue around pkgdown changes since this PR was originally proposed).