Deprecated functions clutter the auto-complete

[AltfunsMA]

Feature request:

Either remove deprecated functions from the package or make them alias the currently preferred function with a warning message that can be optionally silenced.

Rationale

I have used janitor for a few years now and it's a great package, but I don't use it enough that I'm not surprised/frustrated by autocomplete always showing functions that have long been deprecated and will throw an error.

Why export functions that do not work? I guess it'll help those who are running old scripts; but it should be fairly obvious what has happened even without the friendly reminder that these functions are now deprecated.

Why not use those as aliases? I find the old versions (e.g. janitor::add_totals_col()) far easier to remember than the new ones (e.g. janitor::adorn_totals("cols")).

An example from dplyr is illustrative. mutate_at and co. may be superseded in favour of across syntax; but if I feel like typing mutate_if(is.numeric, as.character) because it's simpler than mutate(across(where(is.numeric), as.character) I can still do it.

[billdenney]

It is an interesting idea to somehow hide deprecated functions from auto-complete, but that is outside the scope of what janitor can do. (I'd guess that would either be a base R request or an Rstudio request.) I don't think that's exactly what you're asking here, but it's something to think about pushing upstream.

I understand the frustration of changing interfaces. While @sfirke can speak to this better than I can, the rationale of changing the functions was to make it so that all tabyl modifiers start with adorn_* to have a consistent interface. And, the rationale for keeping the deprecated functions around is because old code stays in archives for a long time. Giving a clear error message saying "your code is old and doesn't work, here is how to fix it" is the reason for keeping those functions around.

For my personal use, I recently had to pull up some ~6 year-old code. The project wanted to refresh that code with newer methods and then make some updates to the data for the analysis. Many parts of the code didn't work out of the box, but the error messages that showed me exactly where and how to make the modifications were helpful to me.

So, while I don't think that this is likely to change, hopefully understanding the rationale for it makes it more palatable.

[AltfunsMA]

Thank you. I do appreciate the explanation but it still leaves me unconvinced that making deprecated functions error is a good choice in this case. I haven't experienced this particular problem with any other package, and I'm trying to respectfully make a case here that it is bad practice. Pushing it upstream would not really solve the issue.

If you're worried about old code, aliasing is much better, particularly since this is a purely stylistic choice. It's good to have a consistent syntax, but you would use less code to make, say, janitor::add_totals_col() simply call janitor::adorn_totals("cols") as you currently do for throwing an error. Maybe you're aware of some maintenance overheads I'm not?

If you feel it is absolutely essential that users of your package learn the new interface, you can certainly keep the warning. Again, it would merely involve changing the current lifecycle::deprecate_stop to lifecycle::deprecate_warn.

Those who have learnt the new syntax will not even notice it. And you'll avoid forcing that person running 6-year old code make changes that are purely stylistic.

As to whether the stylistic change is so fundamentally better, I personally find that forcing people to type quotation marks is the most un-R thing one can do. ;P

[sfirke]

Re: the titular issue of cluttering the namespace, I think the only fix there is to remove the functions entirely. Which might be reasonable: they were first deprecated over 5 years ago in 1.1, then hard-deprecated in 2.0.0 in Spring 2020. But per Bill's comment, and the fact that you're seeing the error messages from old code, keeping the deprecated functions around to redirect people to the replacement functions makes it easier for them.

I don't know that either is a huge deal: you still need to type adorn_ to distinguish those functions before you can autocomplete, and there aren't very many historical users from before tabyl and adorn_ came out in 1.0.0.

I'm inclined to leave things as they are, more than anything because any developer energy I have for this package should go toward a few worthy outstanding issues instead. Maybe we'll drop those deprecated functions in 3.0, though - we hadn't thought that far ahead.

If you feel it is absolutely essential that users of your package learn the new interface, you can certainly keep the warning. Again, it would merely involve changing the current lifecycle::deprecate_stop to lifecycle::deprecate_warn.

I could see following this for future deprecations.

[sfirke]

On a somewhat related note: today I'm researching setting up docker containers to run scheduled R tasks that I want to "just work" indefinitely. Using the approach here, you can lock a specific version of a package with remotes::install_version or MRAN. This is my plan for avoiding getting inconvenienced by software changes like this - a user could use janitor 0.3.0 forever if they want.

[sfirke]

I edited my comment above to clarify that add_totals_col was first deprecated in 2017. It threw a warning for three years before we had it fail instead starting in 2.0.0. So given this longer timeframe (>5 years), maybe we should remove these functions entirely in the next CRAN release.