Skip to content

Revised model documentation #456

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

Merged
merged 85 commits into from
Jun 25, 2021
Merged

Revised model documentation #456

merged 85 commits into from
Jun 25, 2021

Conversation

topepo
Copy link
Member

@topepo topepo commented Mar 25, 2021

This restructures the model documentation files so that

  • Each engine has a separate man file (instead of being in the "Engine Details" subsection)
  • Engines from parsnip-adjacent packages are automatically discovered and linked if they use the same conventions.
  • The engines have more information than before (e.g., preprocessing requirements, references, etc).

Each engine-specific man file links to a Rmd file in man/rmd. The naming convention of the man pages is critical for finding them automatically.

Draft PR: only changes for linear_reg() and you'll need to run pak::pak("tidymodels/multilevelmod@doc-test") to see how other engines from other packages show up.

Copy link
Member

@DavisVaughan DavisVaughan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea! Mainly we'd just need some good documentation about how this works (would also be nice to mention that _ gets subbed for - when going from model name -> file name)

Copy link
Member

@hfrick hfrick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to get to an engine help page directly using its name? You might want to do that when you have already decided on the engine and don't want to click through from ?linear_reg. I tried ?details-linear-reg-glmnet but it didn't like the -. Could we use _ or does that break something?

@topepo
Copy link
Member Author

topepo commented Mar 31, 2021

Feedback from the tidymodels and tidyverse group meetings:

  • Mention that the engines that are shown in the list are from loaded packages.
  • We should list the names of other packages that are known to have engines for the model.
  • We should keep an internal list of known packages with relevant engines and, if they are installed, show the engines anyway.
  • Include the package name in the bulleted list.
  • Add a "learn more at tidymodels.org" note on the pages (and in references)
  • We can have a list of excluded packages that we know will never have the details files (e.g. survival, glmnet, etc). We should test the speed of the loading on a windows machine with a libPath on a network drive (or similar slow config).
  • The details Rd files have names like details-linear-reg-glmnet. These should use underscores instead for easier autocomplete and simpler code.

@juliasilge
Copy link
Member

What a set of changes 😅 but this is looking really great! 🙌

Other than the questions I have open above, I think the only other thing I want to check on in that we don't have these changes implemented for the survival models. That is as designed, correct?

@topepo topepo marked this pull request as ready for review June 25, 2021 15:24
@topepo topepo merged commit 37b07cb into master Jun 25, 2021
@topepo topepo deleted the doc-test branch June 25, 2021 15:28
@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 Jul 10, 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.

4 participants