Pre-release activities

[donyunardi]

Package level checks

• https://github.com/insightsengineering/teal.modules.general/pull/639
• Review and update:
  • README.md (check example code)
  • NEWS.md
• Run urlchecker::url_check() to identify broken links and fix
• Run R CMD check --as-cran make sure everything pass
• Package Title is not duplicated in Package Description in DESCRIPTION file (e.g. this happens in teal.slice currently)
• All package names in Title and Description fields of DESCRIPTION file are quoted with '
• You have checked the Package Release Template https://github.com/insightsengineering/teal.reporter/pull/205/files
• Make sure all teal.* mentions are lower-cased and quoted
• Make Sure inst/WORDLIST is minimalized
• Make sure each link to our documentation hosted with pkgdown on github pages do not have /main/ in the address but it has /latest/ instead, so we always expose the documentation of the latest release and not what's currently on main branch but not yet released,
• Remove exception in .lintr: indentation_linter = NULL
• Remove @noMd (in favor of Roxygen: list(markdown = TRUE) in DESCRIPTION)

Vignettes level checks

• https://github.com/insightsengineering/teal.modules.general/issues/640
• Sanity check of all vignettes, make sure there is no typo, no wrong format, etc.

Non-exported functions level checks

• https://github.com/insightsengineering/teal.modules.general/issues/641
• Make sure non-exported functions do not have examples

Exported functions level checks

• Review functions:
  • @example tag, make sure it runs, fix if otherwise
  • Make sure functions has @return tag to document the return value
  • no \dontrun tag, replace with if(interactive()) if needed
  • Standard order of roxygen2 tags
  • @title ➡️ @doctype ➡️ @description ➡️ @details ➡️ @rdname ➡️
  • ➡️ @inheritParams ➡️ @params ➡️ @return ➡️ @seealso ➡️ @references ➡️
  • ➡️ @examples ➡️ @export ➡️ @keywords ➡️ @noRd
• Make sure there are no ::: in examples
  • if you need to retain the example that uses :::, use getFromNamespace() function.
• Remove return wrapper if it is the last expression (per NEST guidelines)

Please self-assign by your name next to the module and link to the PR:

• [x] tm_g_response.R @averissimo #666
• [x] tm_file_viewer.R @averissimo #665
• [x] tm_t_crosstable.R @averissimo #664
• [x] tm_g_distribution.R @kartikeyakirar https://github.com/insightsengineering/teal.modules.general/pull/669
• [x] tm_data_table.R @kartikeyakirar https://github.com/insightsengineering/teal.modules.general/pull/667
• [x] tm_missing_data.R @kartikeyakirar https://github.com/insightsengineering/teal.modules.general/pull/668
• [x] tm_g_bivariate.R @kartikeyakirar https://github.com/insightsengineering/teal.modules.general/pull/660
• [x] tm_g_scatterplotmatrix.R @kartikeyakirar https://github.com/insightsengineering/teal.modules.general/pull/661
• [x] tm_outliers.R @kartikeyakirar https://github.com/insightsengineering/teal.modules.general/pull/653
• [x] tm_g_scatterplot.R @kartikeyakirar https://github.com/insightsengineering/teal.modules.general/pull/657
• [x] tm_front_page.R @kartikeyakirar https://github.com/insightsengineering/teal.modules.general/pull/654
• [x] tm_a_pca.R @kartikeyakirar https://github.com/insightsengineering/teal.modules.general/pull/646
• [x] tm_variable_browser.R @kartikeyakirar https://github.com/insightsengineering/teal.modules.general/pull/652
• [x] tm_a_regression.R @kartikeyakirar https://github.com/insightsengineering/teal.modules.general/pull/655
• [x] tm_g_association.R @kartikeyakirar https://github.com/insightsengineering/teal.modules.general/pull/656
• [x] Rest of R files @kartikeyakirar https://github.com/insightsengineering/teal.modules.general/pull/676

extra:

• Add common @return tag #670
• Standard function title format in docs https://github.com/insightsengineering/teal.modules.general/pull/691

Potential removal of dependencies

Consider:

• 643

• [x] Removal of {magrittr} by importing pipe operator from {dplyr} :heavy_check_mark: done
• [x] Other packages that can be easily removed :heavy_check_mark: removed tidyselect as its functions are reexported from dplyr
• Move any of the packages from Depends -> Imports
  • [x] {ggmosaic} :x: impossible (see on issue)
  • [x] {ggplot2} :x: impossible (see on issue)
  • [x] {shinyTree} :heavy_check_mark: done
  • [x] {teal.transform} :x: valid: teal.transform functions should be available to tmg users
  • [x] {teal} :x: valid: teal dependency is standard in all module packages
  • [x] {shiny} :x: valid: like teal, module packages use shiny extensively
• [x] Make sure Config/Needs/verdepcheck and pre-commit config are updated
  • https://github.com/insightsengineering/teal.modules.general/pull/683

Update and Apply the Latest Roxygen Documentation Tags

1. https://github.com/insightsengineering/teal.modules.general/issues/647

Unused functions

• 642

• [x] Identify internal functions that are not used (if they exist)
• Discuss case by case if they should be removed

Replace val_labels with col_labels

• https://github.com/insightsengineering/teal.modules.general/issues/658

Correct linting

• 670

Standardize option Notation

• https://github.com/insightsengineering/teal.modules.general/issues/674

[m7pr]

Hey @insightsengineering/nest-core-dev based on the flow I've observed during other pre-release activities in our packages I allowed myself to divide this card into smaller activities

• Package level checks
• Vignettes level checks
• Exported functions level checks
• Non-exported functions level checks

I hope this will allow more people to be included in the collaboration in this process and will make the review process easier. Please make all PRs to pre-release@main branch, but please do not merge, even when a PR is reviewed and approved. We will merge everything, the moment all PRs to pre-release@main branch are approved and pre-release@main is also approved. My proposition is to have a pre-release@main branch be a branch where we review Package-level checks and then merge all other activities in there, before merging to main.

[averissimo]

Agree! It will require some extra focus when doing exported/non-exported, but I think this will help the review process greatly

[chlebowa]

While looking around for unused functions, I came upon add_facet_labels which is exported but not necessarily should be. Should we look for cases like that as well? Do we have time for such an assessment?

[m7pr]

@chlebowa maybe it's a good comment for https://github.com/insightsengineering/teal.modules.general/pull/659 ? If function is not used in any teal package nor in teal gallery it does not need to be exported.

It is used only in teal.modules.general in tm_g_bivariate so prefixing should be enough and no need for exporting. Examples can be extended for this function with getFromNamespace() (and potentially be removed for CRAN release)

[chlebowa]

Prefixing is only for foreign functions. This function (looks to me) is defined in tmg and only used in tmg functions with no apparent exposure to a teal app developer. A function like that should not be exported.

But perhaps there is another reason for this particular function to be exported?

[m7pr]

Yeah, I think you can remove the export. No need for prefixing if it's from tmg, yes, sorry! And only the examples need to be updated as it will not be exported anymore.

[averissimo]

Agree! How did you find this function? Is it an easy process or heavily manual?

[chlebowa]

I was manually going through all functions and searching their names to see whether and when they are called.

[m7pr]

For the automation, maybe there is a way you can list of functions and then grep it's appearance in a directory that contains all teal packages and teal,gallery. And sort functions by the number of apperances, so that the least frequent one can be examined manually?

[kartikeyakirar]

From the namespace file, we can directly check all the exported functions such as add_facet_labels and get_scatterplotmatrix_stats. However, both of these functions are exported and should be internal. I checked this by searching for these functions on the Github search bar for search in organization .

[chlebowa]

I did use the GH search bar to see if it's used in different packages but that is never the first place that I go. It ignores special characters and chops up queries on white space.

[averissimo]

For documentation simplicity and removal of repetitive @return tags I suggest to:

1. Add to #' @return Object of class `teal_module` to be used in `teal` applications to shared_params
   • R/utils.R
2. Use #' @inherit shared_params return on all the modules

---

We should also add a task to this issueto remove all linter errors.

[kartikeyakirar]

For documentation simplicity and removal of repetitive @return tags I suggest to:

I agree,

[m7pr]

@averissimo would you like to take the lead on your propositions, or do you need our help?

[averissimo]

I'll do it :+1:

[averissimo]

The PR is up and running -> #670. After it's merged we can either

1. Change the @return tag on all of the PRs to use this shared
   • after #670 is merged
2. Merge all documentation PRs and then make a single one that changes all @return to @inherit return shared_params

[m7pr]

I think we can merge #670 into here, and then pull changes to all other PRs that we have for functions

[averissimo]

Moved this comment to it's own issue

• 695

[donyunardi]

All works are completed. Closing,