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

Add default usage and format for data documentation #221

Closed
krlmlr opened this issue Mar 20, 2014 · 19 comments
Closed

Add default usage and format for data documentation #221

krlmlr opened this issue Mar 20, 2014 · 19 comments

Comments

@krlmlr
Copy link
Member

@krlmlr krlmlr commented Mar 20, 2014

@usage can be identical to @name by default, for @format the output of str() could be used -- just like with other non-function objects

@hadley
Copy link
Member

@hadley hadley commented Mar 20, 2014

That's already true?

@krlmlr
Copy link
Member Author

@krlmlr krlmlr commented Mar 20, 2014

Not on my system. I have an example built with most recent roxygen2 (896e347), if I remove @format and@usage they also disappear in the .Rd file.

@hadley
Copy link
Member

@hadley hadley commented Mar 20, 2014

Please provide a example reproducible - see existing tests at https://github.com/klutometis/roxygen/blob/master/tests/testthat/test-doctype.R#L24

@krlmlr
Copy link
Member Author

@krlmlr krlmlr commented Mar 20, 2014

My example is reproducible, but I cannot put it in the format of existing tests. I'm documenting a dataset that resides in an .rda file in the data subdirectory -- perhaps the way I do it is just wrong, in this case I'd appreciate a hint on how to fix this. If this is supposed to work but doesn't, I'll be glad to submit a pull that tests a dummy package.

@hadley
Copy link
Member

@hadley hadley commented Mar 20, 2014

Sorry, I missed the minimal ;) I'm happy to have a look, but can you make a package with just the data and one R file?

@krlmlr
Copy link
Member Author

@krlmlr krlmlr commented Mar 20, 2014

I have removed the stuff not related to the issue at hand from the branch I mentioned earlier. This is now minimal, apart from some description what the data is about.

Please say

hub clone krlmlr/SwissCommunes
cd SwissCommunes
git checkout roxydata

to download.

@hadley
Copy link
Member

@hadley hadley commented Mar 21, 2014

Oh yeah, the problem is that this only works when you're documenting local objects, not objects defined elsewhere.

I think fixing this requires some significant changes. I think we'd need a @objname tag to indicate that you're documenting a named object that exists elsewhere (which is not what @name) currently does. This requires some non-trivial changes to parse_file to use the supplied object name rather than the call.

Alternatively it might be possible to teach the parser to document names (rather than calls). Then you could do:

#' Total births per municipality in Switzerland between 1969 and 2012
#' 
#' Each row contains the number of births for a municipality and
#'   a year.  Not all municipalities are present for each year due to
#'   changes in the definition of the municipalities.
#' @author Swiss Federal Statistical Office
#' ...
SwissBirths

that might be a better solution now that I think of it.

@hadley
Copy link
Member

@hadley hadley commented Mar 21, 2014

I think that would work, but it requires that devtools load data before the code, which makes it a little less appealing. I could make this work though:

#' Total births per municipality in Switzerland between 1969 and 2012
#' 
#' Each row contains the number of births for a municipality and
#'   a year.  Not all municipalities are present for each year due to
#'   changes in the definition of the municipalities.
#' @author Swiss Federal Statistical Office
#' ...
"SwissBirths"

or quote(SwissBirths). What do you think?

@krlmlr
Copy link
Member Author

@krlmlr krlmlr commented Mar 21, 2014

Thanks. For me, the syntax that is easiest to implement is just fine. I like both options.

@wch
Copy link
Member

@wch wch commented Mar 21, 2014

I think the plain string, "SwissBirths" is OK. Are there any drawbacks do doing it that way?

@hadley hadley closed this in b65ae96 Mar 21, 2014
@hadley
Copy link
Member

@hadley hadley commented Mar 21, 2014

Done!

@imanuelcostigan I think this is actually what you were asking for, but I was too dense to realise it.

@imanuelcostigan
Copy link

@imanuelcostigan imanuelcostigan commented Mar 22, 2014

Looks like what I was after. Thanks.

@krlmlr
Copy link
Member Author

@krlmlr krlmlr commented Mar 24, 2014

Thanks! Unfortunately, this does not seem to work for my updated minimal example:

> library(roxygen2); roxygenize('.')
Error: Failure in roxygen block at SwissCommunes-package.R:1
object 'SwissBirths' not found
Execution halted

What am I doing wrong?

@hadley
Copy link
Member

@hadley hadley commented Mar 24, 2014

It won't work with roxygenise because it doesn't know how to load datasets. You'll need to use devtools::document()

@krlmlr
Copy link
Member Author

@krlmlr krlmlr commented Mar 24, 2014

No dice:

> library(devtools)
> document()
Updating SwissCommunes documentation
Loading SwissCommunes
Error: Failure in roxygen block at SwissCommunes-package.R:1
object 'SwissBirths' not found

Whereas:

> library(devtools)
> data(SwissBirths)
> document()
Updating SwissCommunes documentation
Loading SwissCommunes
Error: Failure in roxygen block at SwissCommunes-package.R:1
object 'SwissBirths' not found
> roxygen2::roxygenize()
Updating namespace directives
Writing SwissBirths.Rd

So:

  • It seems that the data must be loaded in the current session for the new feature to work (and when it is, it works flawlessly)
  • Even if the data is loaded, document() seems to fail (r-lib/devtools@df94ccc)

Is this a regression in devtools?

@hadley
Copy link
Member

@hadley hadley commented Mar 24, 2014

Grr, I think it's because the data doesn't actually exist in the package environment - it's in a special lazily loaded env.

@krlmlr
Copy link
Member Author

@krlmlr krlmlr commented Mar 24, 2014

Is there something I can change in the definition of the package to make it work?

@hadley hadley reopened this Mar 27, 2014
hadley added a commit that referenced this issue Mar 27, 2014
@hadley
Copy link
Member

@hadley hadley commented Mar 27, 2014

@krlmlr this now works for me with the roxydata branch of SwissComunes.

@wch Can you think of a better way to find the namespace associated with an environment? (iif it's a package environment)

@hadley hadley closed this Apr 11, 2014
@krlmlr
Copy link
Member Author

@krlmlr krlmlr commented Apr 11, 2014

Thanks, it also works on my system, but only using devtools::document() as you said earlier.

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

Successfully merging a pull request may close this issue.

None yet
4 participants