-
Notifications
You must be signed in to change notification settings - Fork 0
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
v0.1.0 release #94
v0.1.0 release #94
Conversation
…lumn_names` (since column specs are now internal)
…ing a readr function anyways
…t it as data_synch_time during db_update
Restructure data import scripts
…issing in raw data
…en if it is missing
…fication 046 Improve handling of synchronization time
…tion. For shiinymanager, use only one initial role (data frame restriction).
… ones are available
…eview_config, update tests
…ng complete report instead of incomplete one
Fix issues with reports
… our upload limits
static_overview_data |> | ||
dplyr::filter(subject_id %in% r$filtered_subjects) |> | ||
dplyr::arrange(subject_id) |> | ||
dplyr::mutate( | ||
needs_review = subject_id %in% unique(rev_data$summary()$subject_id) | ||
) | ||
) |> | ||
dplyr::arrange(dplyr::desc(needs_review), subject_id) | ||
}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jthompson-arcus this is the only code change in this PR; it ensures that the table on the start page is sorted by review status; all patients that were reviewed will be shown at the bottom now (see also #26 (comment))
…opening the review config modal again
R/mod_review_config.R
Outdated
|
||
modvars$site_selection <- input$site_selection | ||
modvars$region_selection <- input$region_selection | ||
|
||
golem::cat_dev("Selected sites:", modvars$site_selection, "\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, there was another small issue that I came across. Added a small change so that selected regions are preserved after changing the settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM @LDSamson! I tested it out using Arcus' data and everything appears to work as expected. I'll let you merge when ready. I've decided to go ahead and incorporate the changes in this branch into our deployed app too, so please let me know if you make any other changes so I can follow up.
# child of the global environment (this isolates the code in the document | ||
# from the code in this app). | ||
rmarkdown::render(tempReport, output_file = fileinput, | ||
rmarkdown::render(tempReport, output_file = basename(fileinput), | ||
params = params, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jthompson-arcus I suspect that this change could solve your problem of not being able to render PDF reports under Windows, closely related to this bug I reported previously: rstudio/rmarkdown#2556
@aclark02-arcus thanks for checking! I made a few more changes, almost all related to documentation:
I will probably switch this PR to the master branch and I think I will merge it today if no problems occur |
Changes:
@jthompson-arcus and @aclark02-arcus I think it is ready for release. I am verifying it internally now. If you can have a look if this version runs on your pc, that would be great!