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

should top_levels return a vector? #93

Open
sfirke opened this issue Feb 3, 2017 · 3 comments
Open

should top_levels return a vector? #93

sfirke opened this issue Feb 3, 2017 · 3 comments

Comments

@sfirke
Copy link
Owner

sfirke commented Feb 3, 2017

Making it a simpler, smaller function would make it a more useful function and have it better fit our API. Right now it recodes and tabulates. If it just recoded, it could still feed tabyl for the same thing, but also could feed crosstab, and be used in data cleaning, to generate a new column.

I'm pretty sure this is a good idea, and also pretty sure hardly anyone uses this function...

@sfirke
Copy link
Owner Author

sfirke commented Feb 3, 2017

I think it's a sign that to implement this, all that's required is to delete these lines at the end of the function:

  # tabulate grouped variable, then reset name to match input variable name
  result <- tabyl(new_vec, show_na = show_na, sort = sort)
  names(result)[1] <- var_name 
  result

and have it return new_vec instead. (also need to change the parameters and make new tests of course)

@sfirke
Copy link
Owner Author

sfirke commented Feb 4, 2017

As long as I'm on this, is there a better name than top_levels? It gives bottom levels too.
And is this function only in janitor because I use it in my particular use cases with Likert survey data, but really it is out of scope and belongs either in forcats or the internal TNTP package at work?

@sfirke
Copy link
Owner Author

sfirke commented Mar 5, 2018

Yeah, I think this is out of scope for janitor. Not really a big deal either way (removing it or keeping). If tidyverse/forcats#28 gets implemented, I can deprecate and then remove.

This would be better as it returning a vector, that can then be fed to tabyl... but is that worth doing for v 1.0?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant