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

Update docs with description tag #26

Closed
wants to merge 3 commits into from

Conversation

sjspielman
Copy link
Member

While starting AlexsLemonade/OpenScPCA-analysis#797, I noticed that the docs were missing some docs!

In many functions we have multiline description blocks, and it turns out these don't render until you explicitly tell roxygen it's a description block. Otherwise, once roxygen sees a line break, it stops that section until it sees a new tag, omitting anything in between. See here: https://cran.r-project.org/web/packages/roxygen2/vignettes/rd.html#the-introduction

Here, I've added @description all around, including in spots without multiline descriptions since this can't hurt for future-proofing. I also made sure that bullet lists which are supposed to be bullet lists actually render as bullet lists.

It's worth noting that we could make some of these docs bits @details instead, but given how we expect this package to be used, I thought it would be best to keep all that information at the top of the docs so folks don't miss anything (docs "Details" sections appear below "Arguments").

@sjspielman sjspielman requested a review from jashapiro December 20, 2024 17:11
@jashapiro
Copy link
Member

I'm not sure if this is needed?

Looking at the diff, it seems to me that most of the content was as expected in the docs (.Rd files): The first paragraph is already in the description block, and any subsequent paragraphs are in details, which seems fine.

The only place where there is a significant change is in the sweep-clusters function, where the bulleted list caused trouble.

If you do want to add @description statements, I would put them on their own lines, which looks much nicer.

@jashapiro
Copy link
Member

If really want to follow the expected format, we should make the descriptions shorter and the details longer?

@sjspielman
Copy link
Member Author

Looking at the diff, it seems to me that most of the content was as expected in the docs (.Rd files): The first paragraph is already in the description block, and any subsequent paragraphs are in details, which seems fine.

Counterpoint: It turns out this is a great demonstration of "people may miss docs if they have to scroll past arguments".

Comment on lines +6 to +14
#' - The algorithm (`algorithm`)
#'
#' - The weighting scheme (`weighting`)
#'
#' - Number of nearest neighbors (`nn`)
#'
#' - The resolution parameter (`resolution`)
#'
#' - The objective function parameter (`objective_function`).
Copy link
Member

@jashapiro jashapiro Dec 20, 2024

Choose a reason for hiding this comment

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

This formatting does not translate properly; we probably want to make all the docs in this package markdown format by adding Roxygen: list(markdown = TRUE) to the DESCRIPTION file: https://roxygen2.r-lib.org/articles/markdown.html

@sjspielman
Copy link
Member Author

Counterpoint: It turns out this is a great demonstration of "people may miss docs if they have to scroll past arguments".

I want to note that I'm conflicted here! I very much take the point that the docs are there, I just missed them, but how meaningful is it that I missed them is the question I'm not sure of...

@jashapiro
Copy link
Member

Counterpoint: It turns out this is a great demonstration of "people may miss docs if they have to scroll past arguments".

I want to note that I'm conflicted here! I very much take the point that the docs are there, I just missed them, but how meaningful is it that I missed them is the question I'm not sure of...

I think my general thought is that we should probably follow the standard format more closely. If the expected documentation standard is that descriptions should be a short paragraph, we should do that, and put the rest in the details section. I don't think that the usage of this package deviates in any significant way from any other package that would justify doing things much differently.

That may mean some reformatting, and I am not wholly opposed to adding @description and @details; I just don't think we need to do it everywhere as you have done here. (And I would keep them on their own lines for aesthetics.)

I was also partly confused by your initial description of the problem, where you talked about line breaks, but what you meant was paragraph breaks (blank lines).

@sjspielman
Copy link
Member Author

I was also partly confused by your initial description of the problem, where you talked about line breaks, but what you meant was paragraph breaks (blank lines).

Very fair confusion! Indeed I mean paragraph.

I think my general thought is that we should probably follow the standard format more closely. If the expected documentation standard is that descriptions should be a short paragraph, we should do that, and put the rest in the details section. I don't think that the usage of this package deviates in any significant way from any other package that would justify doing things much differently.

Yeah, this is fair. I think it's a reasonable expectation that R users are familiar with how to read R docs, and that indeed there are several sections.

@sjspielman
Copy link
Member Author

Closing in favor of fresh eyes doing this again in #27

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

Successfully merging this pull request may close these issues.

2 participants