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

addDataFrame.Rd documentation RE:date/time format #194

Closed
wants to merge 1 commit into from

Conversation

jasonpott
Copy link

I struggled to interpret the documentation for the addDataFrame function in relation to date time formats when writing xlsx.

Outside of Rmarkdown I rarely see the options() function used to control the output of functions. So it wasn't immediately clear that this was what I was supposed to use to control date formats within xlsx.

I have made a suggestion as to how this might be more explicitly communicated.

I struggled to interpret the documentation for the addDataFrame function in relation to date time formats when writing xlsx.

Outside of Rmarkdown I rarely see the options() function used to control the output of functions. So it wasn't immediately clear that this was what I was supposed to use to control date formats within xlsx.

I have made a suggestion as to how this might be more explicitly communicated.
@colearendt
Copy link
Owner

colearendt commented Mar 17, 2022

Thanks so much for the PR! Unfortunately, this will be overwritten almost immediately by ROxygen - would you mind making the changes here?:

xlsx/R/addDataFrame.R

Lines 45 to 48 in 4ed1d01

#' @examples
#'
#'
#' wb <- createWorkbook()

(It's also possible that we could revisit how this date format is passed, and make it an argument! If that seems interesting, a follow-up issue may be a good idea 😄 )

@jasonpott
Copy link
Author

Thanks for the positive reception.

I'm new to making package PRs so still figuring out the structure of packages. I will make the changes there as you suggest.

I do think that an additional parameter in the createWorkbook or saveWorkbook functions might be a useful addition. It seems unlikely that users would want a different date format in different sheets, I may be mistaken.

@jasonpott
Copy link
Author

Can I clarify that you wish for the changes to be made in [4ed1d01]4ed1d01) I don't seem to be able to edit that file directly.

I can edit in the main branch - shall PR from there?

@colearendt
Copy link
Owner

Closed in favor of #195

@colearendt colearendt closed this Apr 12, 2022
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