Skip to content

Fix tests, allow multi vary across, rename rmd functions to docs#126

Merged
tiffanymtang merged 9 commits intomainfrom
fix-tests
Dec 3, 2022
Merged

Fix tests, allow multi vary across, rename rmd functions to docs#126
tiffanymtang merged 9 commits intomainfrom
fix-tests

Conversation

@tiffanymtang
Copy link
Copy Markdown
Collaborator

@tiffanymtang tiffanymtang commented Dec 1, 2022 •
edited
Loading

  • Remove print.tbl output from snapshot tests
  • Change rmd to docs for consistency. Includes renaming create_doc_template() to init_docs(), create_rmd() to render_docs(), and set_rmd_options() to set_doc_options()
  • Allow for varying multiple DGPs/Methods at once in vary_across

@tiffanymtang tiffanymtang requested a review from jpdunc23 December 2, 2022 06:28
@tiffanymtang tiffanymtang changed the title Remove print.tbl output from snapshot tests Update test and allow multi vary across Dec 2, 2022
@tiffanymtang tiffanymtang changed the title Update test and allow multi vary across Fix tests and allow multi vary across Dec 2, 2022
@jpdunc23
Copy link
Copy Markdown
Contributor

jpdunc23 commented Dec 2, 2022

Looks great!

My only suggestion is to add back create_rmd, create_doc_template, and set_rmd_options for backwards compatibility. These can just be simple wrappers around render_docs, init_docs, and set_doc_options that output a warning, e.g.:

warn("create_rmd is deprecated and will be removed in a future version. Please use render_docs instead.")

In case you hadn't see warn yet, it's in https://github.com/Yu-Group/simChef/blob/main/R/signals.R, and just wraps rlang::warn.

Perhaps better yet, we could use the lifecycle package instead. See here: https://lifecycle.r-lib.org/articles/communicate.html#rename-a-function. If you feel like doing this, great, but no pressure; we can just use warn for now. But if you do go with lifecycle, then let's use "0.1.0" for the when argument to deprecate_warn.

jpdunc23
jpdunc23 requested changes Dec 2, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@jpdunc23 jpdunc23 left a comment

Choose a reason for hiding this comment

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

See #126 (comment)

@tiffanymtang
Copy link
Copy Markdown
Collaborator Author

tiffanymtang commented Dec 3, 2022

Thanks for the suggestions! I added the backwards compatibility using lifecycle

@tiffanymtang tiffanymtang changed the title Fix tests and allow multi vary across Fix tests, allow multi vary across, rename rmd functions to docs Dec 3, 2022
@tiffanymtang tiffanymtang merged commit b5e328a into main Dec 3, 2022
@tiffanymtang tiffanymtang deleted the fix-tests branch December 3, 2022 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jpdunc23 jpdunc23 jpdunc23 requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tiffanymtang @jpdunc23