Closes #3014: Bug fix: derive_vars_dt() rejects user-defined date_imputation with highest_imputation = "D" [create website]#3134
Conversation
manciniedoardo
changed the title
|
|
The website/documentation is available at https://pharmaverse.github.io/admiral/3014_derive_vars_dt_user_defined_date_imp_with_d. |
Co-authored-by: Stefan Bundfuss <80953585+bundfussr@users.noreply.github.com>
…thub.com:pharmaverse/admiral into 3014_derive_vars_dt_user_defined_date_imp_with_d
| #' middle of the possible range: | ||
| #' - The`"first"` and `"last"` keywords allow imputation to the first/last | ||
| #' day/month. They can also be used to impute the year if used in conjunction | ||
| #' with the `min_dates` or `max_dates` arguments (see examples). |
There was a problem hiding this comment.
Is it possible to hyperlink to the examples?
| #' with the `min_dates` or `max_dates` arguments (see examples). | |
| #' with the `min_dates` or `max_dates` arguments [(see examples)](link??). |
There was a problem hiding this comment.
not sure this is a good idea - the argument is inherited by other functions.
There was a problem hiding this comment.
I mean the examples in the docs using min/max args
| "{.arg date_imputation} is set to {.val {date_imputation}}.", | ||
| "Are you sure that with this value you are generating all valid dates?", | ||
| "E.g. {.code date_imputation = 31} would impute \"2020-02\" to \"2020-02-31\",", | ||
| "which is invalid." |
There was a problem hiding this comment.
So just catching up here - this is going to fall over if the date is invalid no matter what?? - Should we just say this imputation produces invalid dates and should be reviewed and corrected??.
The phrase: Are you sure that with this value you are generating all valid dates? implies that this can be circumvented?? but i might be off here?
There was a problem hiding this comment.
fair point, let me update the message.
There was a problem hiding this comment.
actually the message is in admiraldev... :( would have to update it there. but to be fair, it will fall over (error) so you will need to go in and update anyhow? or am i being lazy??
There was a problem hiding this comment.
Turns out I hallucinated - I an definitely edit this text and have improved the error message to print out some of the problem dates.
Co-authored-by: Stefan Bundfuss <80953585+bundfussr@users.noreply.github.com>
…thub.com:pharmaverse/admiral into 3014_derive_vars_dt_user_defined_date_imp_with_d
manciniedoardo
deleted the
3014_derive_vars_dt_user_defined_date_imp_with_d
branch
3 participants
Thank you for your Pull Request! We have developed this task checklist from the Development Process Guide to help with the final steps of the process. Completing the below tasks helps to ensure our reviewers can maximize their time on your code as well as making sure the admiral codebase remains robust and consistent.
Please check off each taskbox as an acknowledgment that you completed the task or check off that it is not relevant to your Pull Request. This checklist is part of the Github Action workflows and the Pull Request will not be merged into the
mainbranch until you have checked off each task.styler::style_file()to style R and Rmd filesinst/cheatsheet/admiral_cheatsheet.pptxand re-upload a PDF and a PNG version of it to the same folder. (The PNG version can be created by taking a screenshot of the PDF version.)devtools::document()so all.Rdfiles in themanfolder and theNAMESPACEfile in the project root are updated appropriatelyNEWS.mdunder the header# admiral (development version)if the changes pertain to a user-facing function (i.e. it has an@exporttag) or documentation aimed at users (rather than developers). A Developer Notes section is available inNEWS.mdfor tracking developer-facing issues.pkgdown::build_site()and check that all affected examples are displayed correctly and that all new functions occur on the "Reference" page.lintr::lint_package()R CMD checklocally and address all errors and warnings -devtools::check()To create the website for the content of this PR add "[create website]" to the title of the pull request.