-
Notifications
You must be signed in to change notification settings - Fork 2
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
Extract applications #288
Extract applications #288
Conversation
…eters of applications in a simulation - Add a function `exportParametersToXLS` to write parameter information into an excel file that can be loaded in MoBi or R - Add a function `writeExcel` that is a wrapper for creating a directory (if not present) and writing to excel file
…tract-applications
Codecov Report
@@ Coverage Diff @@
## develop #288 +/- ##
===========================================
+ Coverage 58.91% 60.71% +1.79%
===========================================
Files 23 23
Lines 1110 1148 +38
===========================================
+ Hits 654 697 +43
+ Misses 456 451 -5
Continue to review full report at Codecov.
|
moleculeNames <- moleculeNames %||% simulation$allFloatingMoleculeNames() | ||
|
||
# Returns an object of class `Application` for each administration event | ||
applications <- unlist(lapply(moleculeNames, \(x)simulation$allApplicationsFor(x)), use.names = FALSE) |
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.
We support R versions > 3.6, and so the anonymous function syntax (\()
) can't be used with these older versions of R.
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.
Why not support at least 4.1? I think this is even written somewhere on BlueSpice that R 4.2 should be used.
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.
Fine by me.
But then you should also bump the needed R version in the DESCRIPTION file.
R/utilities-simulation.R
Outdated
#' @param moleculeNames Names of the molecules for which administrations the | ||
#' parameters will be returned. If `NUll`(default), applications for all |
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.
Something is missing in this first sentence.
administrations of the parameters?
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.
The parameters of the applications of the molecules... Don't really know how to properly phrase it.
R/utilities-parameters.R
Outdated
# has been provided | ||
data <- list(output) | ||
if (!is.null(sheet)) { | ||
validateIsOfLength(sheet, 1) |
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.
All validations should happen early on to avoid failing after expensive computations.
You can move this to the top with the addition of , nullAllowed = TRUE
.
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.
Moved
Co-authored-by: Indrajeet Patil <[email protected]>
Co-authored-by: Indrajeet Patil <[email protected]>
Looking good. One last thing: Can you please also make a note of all these changes in NEWS.md? Including the change in needed R version. |
NEWS updated |
* Create V4 (#255) * - Update data-handling vignette - Function `loadObservedData()` requires a `ProjectConfiguration` instead of a `ScenarioConfiguration`. * Remove unused vignettes * Update AppVeyor to R4.2.0 * Increment version to v4.0.0 * Cleanup for v4 * Update website * Update reference index * Removed steady-state function and using ospsuite.parameteridentification (#261) * Removed steady-state function and using ospsuite.parameteridentification * Update NEWS.md Co-authored-by: Indrajeet Patil <[email protected]> * Removed unused message * Update documentation * Update appveyor.yml * Fix examples Co-authored-by: Indrajeet Patil <[email protected]> * Add functions to create plotting-related configs (#265) - [x] Closes #262 - [x] Closes #263 - [x] Closes #264 - [x] Closes #225 - [x] docs and NEWS - [x] tests * Following guidelines for internal function names (#266) * Added simulationRunOptions parameter (#268) * Added simulationRunOptions parameter * Remove `hash` from imports Adds `stringr` and `labeling` to imports Remove "return" from an empty switch statement Missing documented arguments * Update to roxygen2 7.2.1 (#273) * Update to roxygen2 7.2.1 * .rd files * Update exportSteadyStateToXLS.Rd * Don't write to sheets when there is no data (#274) Closes #272 * SensitivityAnalysis - use "by path" methods (#271) * Use unlist * Assign condition to a variable * Skipping tests that rely on PK-Sim * fix `createEsqlabsPlotConfiguration()` Co-authored-by: Indrajeet Patil <[email protected]> * Follow coding convention for source code filenames (#275) * remove empty section from reference page * Fix failing sensitivity-calculation after ospsuite update (#278) * Fix failing sensitivity-calculation after ospsuite update * Update rd * Add tests for utilities (#280) * Add tests for file utilities * snapshot paths have changed? * skip for now * try * problematic test? * Update test-utilities-file.R * Update test-utilities-file.R * Update test-utilities-file.R * Test for `compareWithNA()` * rudimentary test for `ScenarioConfiguration` * Don't skip tests on CI anymore (#283) * Update README (#282) * Update README Closes #281 * Update README.md * Tests for color utilities and global settings (#285) * Tests for color utilities * tests for global settings * Use functions for error messages * Additional tests for utilities (#287) * Additional tests for utilities * test for `compareSimulationParameters()` * tweak tests for settings * Test for `sampleRandomValue()` * Address review comments * remove `expect_s3_class()` for R6 * Extract applications (#288) * - Add a function `getAllApplicationParameters` that returns all parameters of applications in a simulation - Add a function `exportParametersToXLS` to write parameter information into an excel file that can be loaded in MoBi or R - Add a function `writeExcel` that is a wrapper for creating a directory (if not present) and writing to excel file * Update documentation * Update R/utilities-simulation.R Co-authored-by: Indrajeet Patil <[email protected]> * Update R/utilities-parameters.R Co-authored-by: Indrajeet Patil <[email protected]> * Address review comments * Update required R version * Update NEWS Co-authored-by: Indrajeet Patil <[email protected]> * Tests for shiny apps (#290) * Get rid of a few lints (#291) * also not used * - Add default project configuration file (#293) - Fix documentation for the simulationTime argument of ScenarioConfiguration - Fix example for setParameterValuesByPathWithCondition - If simulation time is not defined in the ScenarioConfiguration, use output time schema from the simulation * More tests for utilities (#294) * More tests for utilities * use github version * skip for now * Test for `executeInParallel()` * Try shipping binary * Revert "Try shipping binary" This reverts commit 72dda5d. * Update documentation (#298) * More tests for scenario config (#296) * more tests for scenario config * to shut up codecov * Get rid of `.getNetHashCode()` Closes #297 * Added example project * Add logo for the package (#299) Stickers are big in the R community, and a good way to market ourselves at conferences and meetings. Print them, and distribute them, so that people can put them on their laptops. cc @StephanSchaller * - Remove unused test files - Styler * Delete modeling_environment.R unneeded for the esqlabsR purposes * Documenting data handling and running simulations (#301) * Updated the introduction part of the vignette * Data handling vignette * Vignette on running simulations * Propagated vignette updates to the /docs folder * Updated links to the scenario file to avoid duplicated files * style * style changes to vignettes/data-handling.Rmd Co-authored-by: Indrajeet Patil <[email protected]> * Restored _snaps files that were mistakenly removed * wording in introduction Co-authored-by: Pavel Balazki <[email protected]> * link to aciclovir model Co-authored-by: Pavel Balazki <[email protected]> * Removed function reference, moved installation and contribution to README.md only * style changes README.md Co-authored-by: Indrajeet Patil <[email protected]> * Removed auto-generated testthat file * Update vignettes/introduction-esqlabsr.Rmd * Changes to data-handling.Rmd Co-authored-by: Indrajeet Patil <[email protected]> Co-authored-by: Pavel Balazki <[email protected]> * Better explanation of the configuration Excel files * Restructuring of existing documentation, a clear description of no-code approach * Run tests for Shiny apps (#303) * Simulations vignette with references to all configuration files and first lines of output * Fixed link to the Scenarios.xlsx file * Visualization vignette * Sensitivity vignette * Add getMoleculeNameFromQuantity() (#307) * Add getMoleculeNameFromQuantity() * Apply suggestions from code review Co-authored-by: Indrajeet Patil <[email protected]> Co-authored-by: Indrajeet Patil <[email protected]> * Changed plot sizes * Added troubleshooting sections * Apply suggestions from code review style changes Co-authored-by: Indrajeet Patil <[email protected]> * Apply suggestions from code review style changes Co-authored-by: Indrajeet Patil <[email protected]> * Renamed chunks to follow style guide * Added time profiles to sensitivity analysis * Update vignettes/sensitivity-analysis.Rmd * Added link to time profiles * Updated the HTML docs to match the RMD docs (#309) * Updated the HTML docs to match the RMD docs * correct typos Co-authored-by: Indrajeet Patil <[email protected]> * Suggestions for vignettes (#315) Co-authored-by: Johanna Eitel <[email protected]> * Fixes initializeSimulation() when additionalParams is empty but not NULL #313 (#314) * Added script for installation * - List for packages that will be installed from CRAN - Add "purr" explicitly to be installed from CRAN * - Add jsonlite to be installed * - Change defaults for `updatePackages` argument to `FALSE` - Change defaults for `developerVersion` to `TRUE` - Remove redundant test for PK-Sim connection * Add vignette for shiny apps (#316) * Add vignette for shiny apps * Added header, minor style changes * Add ShinyApp link to the introduction Some minor changes to the vignette Co-authored-by: Johanna Eitel <[email protected]> Co-authored-by: Sergei Vavilov <[email protected]> Co-authored-by: Pavel Balazki <[email protected]> * Update website (#327) * - Remove RData - Update readme * Update docs Co-authored-by: Indrajeet Patil <[email protected]> Co-authored-by: Johanna Eitel <[email protected]> Co-authored-by: Sergei Vavilov <[email protected]> Co-authored-by: Johanna Eitel <[email protected]>
getAllApplicationParameters
that returns all parameters of applications in a simulationexportParametersToXLS
to write parameter information into an excel file that can be loaded in MoBi or RwriteExcel
that is a wrapper for creating a directory (if not present) and writing to excel file