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 vignettes #705

Merged
merged 8 commits into from
Jul 29, 2022
Merged

update vignettes #705

merged 8 commits into from
Jul 29, 2022

Conversation

mhallal1
Copy link
Collaborator

@mhallal1 mhallal1 commented Jul 26, 2022

part of #700

  1. transferred join_keys and preprocessing data vignettes to teal.data
  2. Replaced instances as [XXX()] with XXX
  3. Removed the long printed datasets when possible and the unnecessary messages

@github-actions
Copy link
Contributor

github-actions bot commented Jul 26, 2022

Code Coverage Summary

Filename                         Stmts    Miss  Cover    Missing
-----------------------------  -------  ------  -------  -------------------------------------------
R/default_filter.R                   7       7  0.00%    17-27
R/dummy_functions.R                 74      61  17.57%   12-95
R/example_module.R                  17      17  0.00%    19-35
R/get_rcode_utils.R                 52       2  96.15%   94, 99
R/get_rcode.R                      131      51  61.07%   66, 73, 78, 139-148, 186, 212-261
R/include_css_js.R                  24       0  100.00%
R/init.R                            39      21  46.15%   171, 182-183, 236-257
R/log_app_usage.R                   38      38  0.00%    34-119
R/logging.R                         13      13  0.00%    11-28
R/module_nested_tabs.R             113      16  85.84%   57, 96, 100-117, 163, 212, 257
R/module_tabs_with_filters.R        65       0  100.00%
R/module_teal_with_splash.R         33       2  93.94%   62, 74
R/module_teal.R                    119      20  83.19%   49, 52, 142-143, 156-162, 168-174, 197, 227
R/modules_debugging.R               19      19  0.00%    41-60
R/modules.R                        115       9  92.17%   218, 423-448
R/reporter_previewer_module.R       12       2  83.33%   18, 22
R/show_rcode_modal.R                20      20  0.00%    17-38
R/utils.R                            6       0  100.00%
R/validations.R                     62      39  37.10%   103-355
R/zzz.R                             11       7  36.36%   3-14
TOTAL                              970     344  64.54%

Results for commit: 12e45bf

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@github-actions
Copy link
Contributor

github-actions bot commented Jul 26, 2022

Unit Tests Summary

    1 files    10 suites   15s ⏱️
107 tests 107 ✔️ 0 💤 0
207 runs  207 ✔️ 0 💤 0

Results for commit 2a54a1d.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@nikolas-burkoff nikolas-burkoff left a comment

Choose a reason for hiding this comment

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

  • You need to update the articles list in the pkgdown_yml file

  • At the bottom of the including -x-data-in-teal vignettes we should have a link to here

Comment on lines 29 to 39
and dataset connector. These can be specified on the most general dataset constructor `dataset` as
shown below.

```{r}
```{r, message=FALSE}
library(teal)
library(scda)

# specify keys x and y on a general dataset
dataset("x", data.frame(x = c(1, 1:9), y = 2:11, z = 1), keys = c("x", "y"))

# or for ADSL
# specify keys for ADSL using dataset
adsl <- synthetic_cdisc_data("latest")$adsl
dataset("ADSL", adsl, keys = c("STUDYID", "USUBJID"))
dataset_adsl <- dataset("ADSL", adsl, keys = c("STUDYID", "USUBJID"))
class(dataset_adsl)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really want users to care about this? Surely we want them to use cdisc_dataset (so they get a parent) - and they can specify a keys argument there?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

only cdisc_dataset example is kept.


Note that it is assumed that join keys are symmetric, i.e. `join_key("x", "y", "x_col" = "y_col")` will enable merge
from "x" to "y" and vice versa.
If you want to learn more about basics of join keys, please refer to `vignette("join-keys", package = "teal.data")`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want it to be a vignette call or a link to the github pages?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

replaced by this:

For more information about preprocessing, reproducibility, relationships between datasets and DDL, please refer to the `teal.data.` [vignette](https://insightsengineering.github.io/teal.data)

@@ -38,7 +38,7 @@ For more information, see documentation in `teal.data`.
To learn more about DDL, visit `vignette("delayed-data-loading", package = "teal.data")`. All the features of DDL
Copy link
Contributor

Choose a reason for hiding this comment

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

we should also mention specifying relationships between datasets can be included - see join_keys vignette for details

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For more information about preprocessing, reproducibility, relationships between datasets and DDL, please refer to the `teal.data.` [vignette](https://insightsengineering.github.io/teal.data)

added


```{r}
```{r, eval = FALSE}
Copy link
Contributor

Choose a reason for hiding this comment

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

line 41 there's a space before the comma. And I think in line 39 we should refer to tmh specifically as modules which do work with MAE data

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@nikolas-burkoff nikolas-burkoff left a comment

Choose a reason for hiding this comment

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

Looking good - a few tweaks and this can go in

@@ -76,3 +76,5 @@ if (interactive()) {
shinyApp(app$ui, app$server)
}
```

For more information about preprocessing, reproducibility, relationships between datasets and DDL, please refer to the `teal.data.` [vignette](https://insightsengineering.github.io/teal.data)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For more information about preprocessing, reproducibility, relationships between datasets and DDL, please refer to the `teal.data.` [vignette](https://insightsengineering.github.io/teal.data)
For more information about preprocessing, reproducibility, relationships between datasets and DDL, please refer to the [`teal.data` package](https://insightsengineering.github.io/teal.data)

Copy link
Contributor

Choose a reason for hiding this comment

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

Or point explicitly to the vignette...
image

Copy link
Contributor

Choose a reason for hiding this comment

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

Same in other vignettes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

@@ -37,5 +37,7 @@ if (interactive()) {
```

The filter panel supports `MAE` data out of the box, but `teal` itself does not guarantee that any module will work with `MAE` data the same way it works with other types of data (e.g. `ADaM`) because `MAE` has a unique structure that needs to be considered when developing a module.
The package [`teal.modules.hemres`](https://github.com/insightsengineering/teal.modules.hermes) has been specifically developed for the
Copy link
Contributor

Choose a reason for hiding this comment

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

Refer to github pages not repo?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

vignettes/preprocessing-data.Rmd Show resolved Hide resolved
@mhallal1 mhallal1 merged commit 845d75f into main Jul 29, 2022
@mhallal1 mhallal1 deleted the 700_vignettes@main branch July 29, 2022 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants