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

93 create vignette for tcplplot #114

Merged
merged 4 commits into from
Aug 8, 2023
Merged

Conversation

Ashley-Ko
Copy link
Contributor

  • Added new section called "Plotting" to the data retrieval vignette with three subsections for pdf, jpg, and console output options.
  • Included text from tcpl.R and manuscript to describe the functionality of the tcplPlot package and its parameters.
  • Kept existing mc5 plots.
  • Created three individual examples to illustrate the differences in multi, verbose, and output parameter types.

- Includes images of example plots
- Created section for Plotting
- Added text to describe section
- Examples plots for pdf, jpg, and console output types and multi t/f and verbose t/f options.
…e "Plotting" section to included bullet points, rearranged plot order, and formatted package and variable names.
Copy link
Collaborator

@madison-feshuk madison-feshuk left a comment

Choose a reason for hiding this comment

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

Looks great!

Copy link
Contributor

@sedavid01 sedavid01 left a comment

Choose a reason for hiding this comment

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

Overall looks good. Though there are a few comments to consider in the descriptions. Note, these are suggestions to clarify arguments and use cases so they are minor in nature and should not necessarily block approval of this PR.

vignettes/Data_retrieval.Rmd Outdated Show resolved Hide resolved
vignettes/Data_retrieval.Rmd Outdated Show resolved Hide resolved

- The `verbose` parameter results in a plot that includes a table containing potency and model performance metrics; `verbose = FALSE` is default and the only option in console outputs.

- The `nrow` parameter specifies the number of rows in the multiple plots per page; this is 2 by default. The `ncol` parameter specifies the number of columns in the multiple plots per page; this is 3 by default. If `verbose = FALSE`, `ncol` is 2. `Nrow` and `ncol` can customize the number of plots included per page.
Copy link
Contributor

Choose a reason for hiding this comment

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

So, is there a maximum number of rows and columns that a user can set the output plots to be? That is, is the default multi plot 6 or 4 depending on the verbose parameter, then if a user wants more than that they can add it OR can users only define fewer plots displayed on the output multi plot? Basically what is the behavior of using multi, verbose, and nrow/ncol arguments together? Are there lower and upper bounds on various arguments being used together.

(Note: There does not need to be a whole lot of information but a concise summary of interdependencies/restrictions on these functional arguments may help users when they get started with tcplPlot.)

Copy link
Contributor Author

@Ashley-Ko Ashley-Ko Aug 3, 2023

Choose a reason for hiding this comment

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

@madison-feshuk @brown-jason Is there an upper limit to nrow or ncol?

Copy link
Collaborator

Choose a reason for hiding this comment

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

there is no real limit that we've programmed into the function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It appears that exceeding more than two rows does not reduce the plot size, but rather increases the length of the page. (ncol increases page width) An upper bounds is forced by ggsave. I got this error when trying to run nrow = 10 or ncol = 9

Error in `ggsave()`:
! Dimensions exceed 50 inches (`height` and `width` are specified in
  inches not pixels).
ℹ If you're sure you want a plot that big, use `limitsize = FALSE`.

Is it worth mentioning an upper bounds if there is not a hard coded limit?

  that verbose is before multi.
- Updated the multi parameter description to
  reflect default values being set by verbose
  where  multi = True.
- Added the hard coded lower limit of 0 and
  technical limit of nrow = 9 and ncol = 7 to
  nrow/ncol description .
Copy link
Collaborator

@madison-feshuk madison-feshuk left a comment

Choose a reason for hiding this comment

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

Updates look good to me!

@Ashley-Ko Ashley-Ko merged commit 59e5d07 into main Aug 8, 2023
1 check passed
@Ashley-Ko Ashley-Ko deleted the 93-create-vignette-for-tcplplot branch August 8, 2023 15:02
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.

Create vignette for tcplPlot
4 participants