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

New Check: All .Rd files have \value tags #157

Closed
mpadge opened this issue Aug 19, 2022 · 5 comments
Closed

New Check: All .Rd files have \value tags #157

mpadge opened this issue Aug 19, 2022 · 5 comments
Assignees
Labels
enhancement New feature or request 🔍 Check Idea

Comments

@mpadge
Copy link
Member

mpadge commented Aug 19, 2022

This is now caught automatically on CRAN incoming checks, so would be useful to have here, via checks for roxygen2 @return tags. That in turn would require installation of package, so affects #61

@mpadge
Copy link
Member Author

mpadge commented Aug 24, 2022

Re-opening to address this issue identified by @santikka ropensci/software-review#554 (comment)

@mpadge mpadge reopened this Aug 24, 2022
@santikka
Copy link

@mpadge A similar issue arises when defining a new method and its instances for different classes which share a return value, for example in dynamite there is new method get_code for both dynamiteformula and dynamitefit objects, which are all documented together. Defining a @return tag in the particular method instances will create duplicate entries under Value in the main method documentation page.

mpadge added a commit that referenced this issue Aug 24, 2022
That reduces positives down to only those objects actually defined as functions
@mpadge mpadge closed this as completed in a598643 Aug 24, 2022
@mpadge
Copy link
Member Author

mpadge commented Aug 24, 2022

Thanks again for your help @santikka. The previous approach directly parsed the roxygen2 blocks; the above commit replaces that with direct parsing of the .Rd files via tools::parse_Rd. The whole test attempts to replicate how the CRAN check works, without having any way of knowing exactly how they implement it. Their one would, however, definitely use parse_Rd, so this updated version should come a lot closer.

The previous, roxygen2-based one gave this list from dynamite:

  • as_draws.dynamitefit
  • coef.dynamitefit
  • dynamite
  • formula.dynamitefit
  • aux
  • +.dynamiteformula
  • print.dynamiteformula
  • get_priors.dynamiteformula
  • get_priors.dynamitefit
  • get_code.dynamiteformula
  • get_code.dynamitefit
  • get_data.dynamiteformula
  • get_data.dynamitefit
  • fitted.dynamitefit
  • mcmc_diagnostics.dynamitefit
  • plot_nus
  • predict.dynamitefit
  • print.dynamitefit
  • summary.dynamitefit

The above commit reduces that to what i hope would reflect your expectations:

  • coef.dynamitefit
  • dynamite
  • fitted.dynamitefit
  • formula.dynamitefit
  • plot_nus
  • +.dynamiteformula
  • predict.dynamitefit
  • print.dynamitefit
  • print.dynamiteformula
  • summary.dynamitefit

I've closed this again now, but please comment on whether you now judge the results to appropriately reflect the docs of dynamite. Thank you so much for all the help youi've given us! I'll update the "allcontributors" section of the README straight away to include you.

@mpadge
Copy link
Member Author

mpadge commented Aug 24, 2022

@santikka I've updated the bot report for your submission to reduce listed funtions to the second, shorter version above.

@santikka
Copy link

@mpadge Thanks! That list looks correct, because we hadn't documented the return value for almost any of the generic methods for dynamitefit objects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request 🔍 Check Idea
Projects
None yet
Development

No branches or pull requests

2 participants