Adding tools for migrating from `@param` docs

[dgkf]

An initial pass at some conversion tools. For now just covering @param tags.

Still a bit of a work in progress, but wanted to share what I was able to come up with with a couple hours of toying with a conversion tool. @danielinteractive I think it's in a reasonable enough state that you could throw it at some code bases to see how it does.

As an example, converting mmrm would look like:

roxytypes::convert_params("(`{type}`)\\cr {description}")
# Examples (3 of 221)
# ── satterthwaite.R ────────────────────────────────────────────────────────────
# 225    -#' @return Usually the calculation is returning `2 * E / (E - n)` where
#     225+#' @typedreturn
#     226+#'   Usually the calculation is returning `2 * E / (E - n)` where
# 226 227 #'   `E` is the sum of `t / (t - 2)` over all `t_stat_df` values `t`.
# 
# ── utils-nse.R ────────────────────────────────────────────────────────────────
# 33   -#' @param call,expr (`language`)\cr a language object to flatten.
#    33+#' @typed call,expr: language
#    34+#'   a language object to flatten.
# 
# ── utils.R ────────────────────────────────────────────────────────────────────
# 148    -#' @param optimizer_fun (`function` or `list` of `function`)\cr altern…
#     148+#' @typed optimizer_fun:
#     149+#'   (`function` or `list` of `function`)\cr alternatively to `optimiz…
# 149 150 #'   an optimizer function or a list of optimizer functions can be pas…
# 
# Continue to modifying files with edits?
# 
# y: Yes
# n: No
# #: Preview a few more (default 3)
# 
# Selection: 

It seems to do a pretty good job. Note that the last example that is shown does not get automatically converted because we're looking for a single (`{type}`) and it uses a mix of code blocks and non code (`function` or `list` of `function`).

Right now roxytypes doesn't really handle sum type definitions like this so I think this is the "right" behavior for now, but maybe in the future this could be handled more gracefully.

[danielinteractive]

Great, thanks Doug! Cool stuff. I will give it a try. Regarding the last preview example, I guess it would just be a matter of coming up with the "right" regexp that covers the "or" case right?

[danielinteractive]

So for the mmrm package I just tried this now. I am surprised a bit that it started installation of the mmrm package when I ask it to convert the params: installing *source* package ‘mmrm’ ... and it starts compiling the sources etc. Is it strictly necessary to install the package under development with roxytypes?

[danielinteractive]

Another improvement idea would be to automatically populate the right stuff in the DESCRIPTION file when doing this conversion. Basically as a user I would hope that everything needed is being set up for me automatically. Otherwise, when just changing the roxygen chunks and running document, it won't do the right thing, e.g. omitting argument documentation altogether.

[dgkf]

I am surprised a bit that it started installation of the mmrm package when I ask it to convert the params:

Hmm - I'm surprised too. It might be the roxygen2 internals prompting it. If you just do roxygen2::roxygenize, does it also install then? (the internal function I use is roxygen2::parse_package() if you want to test exactly what might be prompting it)

I've noticed that roxygen2 will occasionally rebuild, and it's not clear exactly what causes it. I've seen it a couple times doing roxygen2::roxygenize on the mmrm codebase.

It didn't do this when I ran it locally, so you might have just gotten unlucky with whatever roxygen2 is doing.

[danielinteractive]

Interesting, but ok, I guess only for packages with compiled source code this might be a bit unwelcome for the users, so we can keep that as is, because it is coming from roxygen2 so we can't control that.

[dgkf]

I'll explore it further. I definitely was not expecting it to rebuild, so I'd wholeheartedly call that undesirable without a user prompt at the very least.

My suspicion is that a solution is going to be a pretty nasty hack around roxygen2 behaviors. If that's the case, I would call this low priority for now.

[dgkf]

Still need to add a lot of tests, but I think core behaviors are pretty much ready to go now.

[danielinteractive]

Cool, thanks Doug for the configuration functionality, that is super helpful. Yeah testing is not trivial for this package, but will be important for maintenance, so will be good to invest there.

[danielinteractive]

Just tried this branch again on mmrm and maybe there is a small bug, because when I say "1" for Yes, modify files it is continue prompting for each edit?... Ah wait, I need to say "y". Maybe if somebody says "1" it should say "you need to enter y, n or ..." ?

[danielinteractive]

And I get now:

[dgkf]

Thanks for testing - I did a major refactor of how all the editing happens yesterday and didn't test super rigorously before committing, so I'm not surprised that there are bugs. I'll be writing tests today so hopefully most of these little hiccups get ironed out as I start to test units.

To revisit a previous topic - When you ran it again on mmrm, did it kick off a package rebuild each time?

[dgkf]

When I say "1" for Yes, modify files it is continue prompting for each edit?... Ah wait, I need to say "y". Maybe if somebody says "1" it should say "you need to enter y, n or ..." ?

Yeah, I'm still trying to work out the best way to write this prompt. I'm not totally satisfied with it. Open to suggestions.

As it is now, you have y for continue, n/q for abort and entering an integer will display that number of edit diffs.

[dgkf]

Other updates:

1. I ran on a fresh mmrm clone and was able to reproduce the build step. To load the package objects and parse formals for default values, this is a necessary step, since the package namespace is needed in order to pull object data. This would be the same if I had run roxygen2::roxygenize, so I think this is an acceptable overhead. Subsequent runs don't rebuild.
2. I reproduced the above error, now fixed.

Still need to write a lot of tests, but please continue reporting any issues you find. I've been doing ad-hoc tests against mmrm and it seems to handle the conversion gracefully now.

[danielinteractive]

Thanks Doug, yeah this matches my experience, the mmrm package only rebuilt once but not after that. So that is ok.