Documentation: Approaching 1.0 final updates for code and documentation (ideas and wishlist)

[bms63]

Hi @pharmaverse/admiral

Hoping this thread will become a running list of ideas, wishes, desires, problems around updates for Vignettes, Documentation and Readme.

BACKSTORY: I put in an issue for updating the generic.Rmd vignette for derive_extreme_events(). However, as Stefan pointed out we should wait till we are more stable to make this update. It got me thinking that we should start a running list of other updates to vignettes, readmes, documentation, etc that we might want to discuss and track.

I like this process of using Discussions to really nail down the work that should be done and then creating an issue that links to the discussion. That way we don't have such vague and difficult issues to tackle.

[bms63]

Update Generic.Rmd

Do we want to add anything to the https://pharmaverse.github.io/admiral/cran-release/articles/generic.html guide on derive_extreme_events()?

Any thoughts on this one @bundfussr ?

Sorry, I missed that one. In principle, yes. However, I would expect that we have to update the vignette significantly once the cleanup is complete. Thus I wouldn't do it now.

Originally posted by @bundfussr in #2015 (comment)

[bms63]

join arguments need some additional discussion in vignette

[bms63]

Records versus Observations

#2066 (comment)

[bms63]

1.0 means 100% test coverage

Once all the code updates are done for 1.0 I think it is worthwhile to bump our test coverage up to 100%

[bundfussr]

I think we can change most of the functions in the "Other" section to internal functions and stop exporting them.

However, I would list all exported function on the "Reference" page. If an exported function is not on the "Reference" page, it could confuse users and health authorities. I would wonder if it is safe to use a function which is not on the "Reference" page.

[bundfussr]

I would restructure the "Reference" page such that it has a similar structure as in the admiral Function Overview. Then the functions with the biggest impact are at the beginning.

[bms63]

Decision: We are going to no longer export developer-focused functions, e.g. assert_db_requirements(). Use internal for kewords. Continue to have a high-level of documentation even if not exported.

• assert_db_requirements() - Remove
• assert_terms() - Remove
• assert_valid_queries() - Remove
• extend_source_datasets() - Remove
• filter_date_sources() - Remove
• format(<basket_select>) - Export, but change keyword to internal
• validate_basket_select() - Remove
• validate_query() - Remove
• chr2vars() - Remove
• signal_duplicate_records() - Export, but change keywords to internal
• extract_duplicate_records() - Export, but change keywords to internal

Issue created for this one: #2220 (comment)

[bms63]

Condense Utilities into only 1 section

[bms63]

Review Example Datasets

[bms63]

Partial Matching for arguments

I believe we have had a few arguments sneak through that are actually not arguments in our functions - either in examples or templates. - just due to typos. I believe R has some partial matching for argument to help deal with this, but we might want to turn this off. There is a way to set this I have been told, but unclear on it. @pharmaverse/admiral any memories?

[bundfussr]

As far as I know, you can't turn it off. But you can set the warnPartialMatchArgs option. Then a warning is issued if partial matching for arguments is used,

[bms63]

where does one set this option?

[bundfussr]

options(warnPartialMatchArgs = TRUE)

[bms63]

place into .Rprofle?

[bundfussr]

Improve "See also"

I would propose to change the usage of the @family tag. At the moment it replicates the section of the "Reference" page. Therefore you can't leave the section by the links provided in the "See also" section.

I would use more than one @family tag. For instance, for derive_summary_records() we could use @family derv_obs and @family derv_sum. The first one lists all derivations adding observations and the second one lists all derivations which summarise like derive_summary_records() and derive_var_merged_summary().

[bms63]

Ummm...I vaguely remember talking about this two years ago with the multiple family tags. I also think there was a reason we had to pin the family and keywords together. Happy to try another way to display information.

[bundfussr]

@bms63 , as far as I remember you suggested to pin the family and keywords tag together but I do not remember the reason.

[bms63]

lol - well I have no memory of why I chose this path. happy to experiment if it improves documentation. maybe I just chose to make it easier as an initial roll out???

[manciniedoardo]

Decision from 10th Nov meeting: consider this update after 1,0 release as it would involve considerable work due to decisions regarding families, reviews of all functions, and new documentation of these families.

[bms63]

I added this into the Roadmap 2.0 discussion with link and will close - #2191 (reply in thread)