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

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

Merged
merged 9 commits into from
Dec 3, 2022

Conversation

tiffanymtang
Copy link
Collaborator

@tiffanymtang tiffanymtang commented Dec 1, 2022

  • 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 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
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.

Copy link
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.

@tiffanymtang
Copy link
Collaborator Author

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants