Add v6 project-snapshot JSON Schema (proposed clean variant) [RFC]#1069
Draft
PavelBal wants to merge 45 commits into
Draft
Add v6 project-snapshot JSON Schema (proposed clean variant) [RFC]#1069PavelBal wants to merge 45 commits into
PavelBal wants to merge 45 commits into
Conversation
Introduce the read-only `modified` flag on `Project`, paired with the existing `validatedSinceMutation` flag. Both are routed through the private `.invalidate()` helper, which the (Chapter 6) `add*` / `remove*` mutation API will call via `.markModified()` after every successful mutation. - R/project.R: add `modified` active field; private `.modified` field; extend `.invalidate()` to set `.modified <- TRUE` (resolves the Chapter 4 TODO marker).
The JSON-array shape (unnamed list, each entry carrying its own
`individualId` / `populationId`) forced every consumer through a
linear scan or the transitional `.extractEntryIds` helper. Flip the
parser to build named lists keyed by id, matching the end-state
shape on `json-as-primary-input-v2`. Applications also gain an
`Application` class attribute so future S3 dispatch can hook in
without re-walking the parser.
Drops the transitional `.findById` and `.extractEntryIds` helpers;
section validators and cross-reference checks now iterate via
`names()` and look entries up by `[[id]]` directly.
- R/project-parse.R: new `.parseIndividuals`, `.parsePopulations`,
`.parseApplications`. Each stamps `class = c("<Section>", "list")`
on parsed entries; numeric fields go through `as.double()`.
- R/project-to-json.R: `.individualsToJson` / `.populationsToJson` /
`.applicationsToJson` re-attach the list key as the matching id
field so JSON round-trip is structurally identical.
- R/scenario-execution.R: drop `.findById`; look up
`project$individuals[[scenario$individualId]]` directly.
`unclass(popData)` in the population branch since the entry now
carries an "Population" class.
- R/validation.R: drop `.extractEntryIds`. `.validateCrossReferences`
reads ids via `names()`; the individual-parameter-sets walk
iterates `names(project$individuals)` and reads
`project$individuals[[id]]$parameterSets`.
- R/utilities-individual.R, R/utilities-population.R: section
validators iterate `names()` instead of `seq_along()`; `.check_no_
duplicates` is no longer needed (named lists cannot collide).
- tests/testthat/test-project-parse.R: assert named-list keys and
`Individual` / `Population` / `Application` class attributes.
- tests/testthat/test-validation.R: fixtures use named lists.
- tests/testthat/test-scenario-execution.R: drop the `.findById`
unit test (helper removed).
Land the Chapter 6 mutation API: every section that the JSON loader populates now has a matching `addX(project, ...)` / `removeX(project, ...)` standalone, plus an R6 delegate of the same name on `Project` so `project$addX(...)` and `addX(project, ...)` are equivalent. Each mutator routes through `project$.markModified()` after a successful change, which flips `modified` to TRUE and clears the `validatedSinceMutation` flag — the integration hook that prevents a stale validation result from letting `runScenarios()` / `createPlots()` short-circuit on a now-invalid project. API surface matches the end-state branch (json-as-primary-input-v2): function names, argument shapes, error/warning text, FK checks, the "warn-then-remove" semantics for `removeX()` on referenced ids, and the auto-removal of empty parameter sets after the last entry is removed. `removeX()` on a missing key warns + no-ops; on a key still referenced elsewhere it emits a `cli::cli_warn()` listing the referrers and proceeds with the removal. The `.warnIfReferenced()` helper in `R/validation.R` walks scenarios for inbound references (individual / population / application / modelParameterSet / outputPath) and the holder section for parameter-set families. Also makes the `Project` section active bindings settable so the end-state-style mutator pattern `project$x <- new` works from inside the standalone functions; metadata bindings (`schemaVersion`, `jsonPath`, `validatedSinceMutation`, `modified`, ...) stay read-only. - R/utilities-scenarios.R: addOutputPath / removeOutputPath, addScenario / removeScenario. - R/utilities-individual.R: addIndividual / removeIndividual, setIndividualParameterSets, addIndividualParameterSet / removeIndividualParameterSet, addIndividualParameterSetEntry / removeIndividualParameterSetEntry. - R/utilities-population.R: addPopulation / removePopulation. - R/applications.R (new): addApplication / removeApplication, setApplicationParameterSets, addApplicationParameterSet / removeApplicationParameterSet, addApplicationParameterSetEntry / removeApplicationParameterSetEntry. - R/utilities-parameters.R: addModelParameterSet / removeModelParameterSet, addModelParameterEntry / removeModelParameterEntry, plus low-level `.addParameterEntry` / `.removeParameterEntry` / `.findParameterEntryIndex` / `.validateParameterEntryArgs` shared by all parameter-set families. - R/observed-data.R: addObservedData (DataSet or config list) / removeObservedData / getObservedDataNames; loadObservedData() extended to merge runtime programmatic DataSets and cache names. - R/utilities-figures.R: addPlot / removePlot, addPlotGrid / removePlotGrid, addDataCombined / removeDataCombined, plus the small `.requireNonEmptyString` / `.namedDotsAsRow` / `.checkDataCombinedEntry` / `.splitPlotIDs` helpers. - R/project.R: section active bindings made settable; private `.programmaticDataSets` / `.observedDataNamesCache` fields for observed-data runtime state; ~30 R6 delegate methods covering every standalone above (addX / removeX / setX), each calling the standalone with `self`. - R/validation.R: `.warnIfReferenced(project, entityType, id)` - shared helper for the remove-with-dangling-reference warning. - NAMESPACE / man/*.Rd: 33 new exports + matching topics. - _pkgdown.yml: new "Programmatic Project mutation" reference section listing every export. - tests/testthat/test-project-mutation-api.R: lifecycle (modified + validatedSinceMutation flips), R6 delegate parity, FK validation snapshots, .warnIfReferenced warnings, JSON round-trip. - tests/testthat/_snaps/project-mutation-api.md: snapshots for addScenario/addIndividual/addOutputPath aborts. - tests/testthat/test-observed-data.R: programmatic sentinel test flipped from "errors" to "skips" since loadObservedData() now silently produces an empty result for orphan sentinels (an in-memory DataSet must be re-attached via addObservedData).
- R/utilities-figures.R: wrap multi-length values in `list()` inside `.namedDotsAsRow()` so `addPlot(..., xValuesLimits = c(0, 100))` produces a single list-column row instead of recycling into N rows. Use `which()` for logical indexing in `removePlot()` and `removePlotGrid()` to drop NA rows from the predicate. - R/observed-data.R: drop the "remove first programmatic sentinel" fallback in `removeObservedData()`. Sentinels written by `addObservedData()` always carry the matching name, so the fallback only fires on hand-edited JSON and risks removing the wrong entry. - R/utilities-parameters.R: reject `units = NA` in `.validateParameterEntryArgs()` so `addModelParameterEntry()` etc. abort with a clear message instead of crashing on `if (nchar(NA) == 0L)`.
- R/utilities-parameters.R: - .removeParameterEntry returns list(parameters, removed) instead of plain set - removeModelParameterEntry returns early when removed is FALSE - R/applications.R: - removeApplicationParameterSetEntry returns early when removed is FALSE - R/utilities-individual.R: - removeIndividualParameterSetEntry returns early when removed is FALSE - tests/testthat/test-project-mutation-api.R: - add cache-invariant test for missing-entry no-op across all three remove functions
- R/utilities-figures.R:
- drop unreachable is.null(project$plots) initializer in addPlot
- addPlotGrid aborts with a clear message when no plots are defined
instead of falling through setdiff and reporting all plotIDs as unknown
- R/utilities-scenarios.R:
- reject NA / non-character / empty FK args in addScenario for
individualId, populationId, applicationProtocol, modelParameterSets,
outputPathIds via inline checkScalarFK / checkVectorFK helpers
- tests/testthat/test-project-mutation-api.R:
- snapshot tests for the new addScenario NA rejection
- snapshot test for addPlotGrid abort when no plots exist
- R/parameter-identification.R: - new file owning PI records, validator, runtime, and mutation API - plain-data PITask, PIParameter, PIOutputMapping with print methods - public runPI(project, piTaskNames, observedData, stopIfParameterNotFound) - addPITask/removePITask, addPIParameter/removePIParameter, addPIOutputMapping/removePIOutputMapping - .parameterIdentificationValidatorAdapter for section validation - soft-deprecation stub for createPITasks() that errors with a pointer to runPI() - R/pi-task-configuration.R, R/utilities-pi.R, R/utilities-pi-configuration.R: - remove legacy R6 PITaskConfiguration, Excel reader, helper utilities - R/project.R: - parameterIdentification active binding (read/write) - delegate methods addPITask/removePITask, addPIParameter/removePIParameter, addPIOutputMapping/removePIOutputMapping - R/project-parse.R: - wire .parsePITasks() into .loadProjectJson() - R/project-to-json.R: - wire .parameterIdentificationToJson() into .projectToJson() - R/validation.R: - register parameterIdentification in .validationSections - cross-reference walk for scenarios and outputPathId references - R/messages.R: - new validation messages for PI cross-references - drop unused errorPI* helpers tied to the removed Excel reader - NAMESPACE: - export runPI, addPITask, removePITask, addPIParameter, removePIParameter, addPIOutputMapping, removePIOutputMapping - S3method print.PITask, print.PIParameter, print.PIOutputMapping - remove PITaskConfiguration, readPITaskConfigurationFromExcel, legacy print methods - inst/extdata/projects/Example/Project.json: - add parameterIdentification section to the example fixture - tests/testthat/data/TestProject/Project.json: - add parameterIdentification section to the test fixture - tests/testthat/test-parameter-identification.R: - new tests for records, parse/serialize, validator, runtime, mutation API - tests/testthat/_snaps/parameter-identification.md: - new snapshots for PI errors and print output - tests/testthat/test-pi-task-configuration.R, tests/testthat/test-utilities-pi-configuration.R, tests/testthat/test-utilities-pi.R: - remove tests for deleted legacy modules - tests/testthat/_snaps/pi-task-configuration.md, tests/testthat/_snaps/utilities-pi-configuration.md: - remove snapshots for deleted legacy modules - tests/testthat/test-project-to-json.R: - cover parameterIdentification round-trip - _pkgdown.yml: - add new PI mutation topics to the reference index - list createPITasks under a deprecated section - vignettes/pi-workflow.Rmd: - banner pointing to runPI(project); legacy chunks marked eval = FALSE - man/addPITask.Rd, man/removePITask.Rd, man/addPIParameter.Rd, man/removePIParameter.Rd, man/addPIOutputMapping.Rd, man/removePIOutputMapping.Rd: - new roxygen-generated topics - man/createPITasks.Rd, man/runPI.Rd: - regenerate for the new signatures and deprecation - man/PITaskConfiguration.Rd, man/readPITaskConfigurationFromExcel.Rd: - remove topics for deleted exports - NEWS.md: - add Internal (work in progress) bullet for the JSON-first PI workflow - drop stale Excel-PI bullet from the unreleased v6 section
- R/validation.R: - validate PITask$scenarios against scenarioNames, mirroring the existing parameter/outputMapping cross-reference checks - R/parameter-identification.R: - switch createPITasks lifecycle badge from deprecated to defunct - man/createPITasks.Rd: - regenerate for the defunct badge - tests/testthat/_snaps/parameter-identification.md: - update runPI snapshot to include the new task-level scenarios error
- inst/extdata/projects/Example/Project.json: - correct outputMapping scaling "Linear" to "lin" - correct configuration ciMethod "LinearApproximation" to "hessian" - tests/testthat/data/TestProject/Project.json: - correct outputMapping scaling "Linear" to "lin" - correct configuration ciMethod "LinearApproximation" to "hessian" - tests/testthat/test-parameter-identification.R: - add end-to-end test running runPI(project) to a PIResult - add failure-path test asserting warning and NULL result - use valid scaling/ciMethod values in record and print tests - tests/testthat/_snaps/parameter-identification.md: - refresh print snapshots for corrected scaling and ciMethod
…-project # Conflicts: # NAMESPACE # R/project.R # tests/testthat/test-utilities-pi.R
- tests/testthat/test-parameter-identification.R:
- add test that a multi-scenario PIParameter builds one shared optimisation variable spanning both simulations
- add test that the same path split into separate PIParameter records stays independent per scenario
- remove skip_if_not_installed("ospsuite.parameteridentification") since it is a hard Imports dependency
- R/utilities-scenarios.R → R/scenarios.R
- R/utilities-individual.R → R/individuals.R
- R/utilities-population.R → R/populations.R
- R/utilities-parameters.R → R/parameters.R
- R/utilities-figures.R → R/plots.R
- R/utilities-data-combined.R → R/data-combined.R
- R/output-paths.R:
- new file. addOutputPath / removeOutputPath plus the output-paths
section validator extracted from R/scenarios.R
- tests/testthat/test-utilities-*.R renamed to mirror sources
- snapshot directories renamed alongside (plots/, populations.md, ...)
- NEWS.md: bullet under ## Internal noting the reorganisation
- R/project.R:
- merge ProjectConfiguration into Project (exported)
- add writable Excel-bridge file fields backed by .filePathsData
- replace piecewise initialize with single projectFilePath arg
- add .markSaved, .getFilePathsData, sync, asList, filePaths accessor
- append ProjectConfiguration() deprecation wrapper
- R/project-parse.R:
- shrink .loadProjectJson to a Project$new shim with path validation
- R/project-to-json.R:
- rewire .filePathsToJson to walk .getFilePathsData
- R/project-excel.R:
- new file consolidating importProjectFromExcel, exportProjectToExcel,
projectStatus, .projectSync, and Excel readers/writers
- soft-deprecation wrappers snapshot/restore/projectConfigurationStatus
- R/utilities-scenarios.R:
- drop createScenarios, .runLegacyScenarios, legacy runScenarios arm
- migrate .parseSimulationTimeIntervals from the deleted file
- R/utilities-population.R:
- migrate .splitCommaString from the deleted file
- R/utilities-project-configuration.R:
- replace createProjectConfiguration body with deprecate_warn forward
- R/utilities-figures.R:
- update roxygen example to use Project$new()
- R/messages.R:
- update user-visible "ProjectConfiguration" strings to "Project"
- R/validation.R:
- relocate isAnyCriticalErrors and validationSummary
- delete legacy R sources:
- project-configuration.R, scenario-configuration.R, legacy-scenario.R
- utilities-scenario-configuration.R, utilities-config-json.R
- validation-all-configurations.R, validation-project-configuration.R
- validation-scenarios.R, validation-individuals.R
- validation-populations.R, validation-plots.R
- validation-models-applications.R, validation-cross-references.R
- delete legacy tests:
- test-project-configuration.R, test-scenario-configuration.R
- test-utilities-scenario-configuration.R, test-utilities-config-json.R
- test-legacy-scenario.R, test-create-scenarios-from-pkml.R
- test-create-plots-from-excel.R
- tests/testthat/test-project.R, test-project-excel.R:
- new tests for merged class and Excel-bridge deprecation wrappers
- tests/testthat/helpers.R:
- reroute testProjectConfiguration to loadProject
- existing tests updated:
- migrate ProjectConfiguration$new callsites to Project$new
- skip Excel-first initProject tests pending Act 2.1 rewrite
- regenerate validation snapshot
- R/project-lifecycle.R:
- rewrite as full lifecycle module
- loadProject calls .warnOnCrossReferenceErrors after construction
- saveProject(project, path): new export; writes Project.json,
calls project$.markSaved()
- createProjectConfiguration / createDefaultProjectConfiguration:
relocate deprecation stubs from utilities-project-configuration.R
- isProjectInitialized: now also recognises Project.json
- initProject(destination, type = c("minimal", "example"),
createExcel = TRUE, overwrite = FALSE): full rewrite copying
template + folder skeleton, optionally producing Excel side-cars
- exampleProjectPath(): new export
- exampleProjectConfigurationPath(): soft-deprecation forwarding
- .projectDirectory(name): new internal helper
- R/scenario-from-pkml.R:
- new file. createScenariosFromPKML(pkmlFilePaths, project, ...) builds
Scenario records into a Project; replaces the deleted
createScenarioConfigurationsFromPKML
- R/utilities-project-configuration.R:
- delete; all surviving content moved to project-lifecycle.R
- inst/extdata/projects/Blank/Project.json:
- new minimal template seeded by initProject(type = "minimal")
- tests/testthat/helpers.R:
- rewrite with_temp_project and local_test_project to use the JSON-
first flow
- tests/testthat/test-project-initialization.R:
- un-skip the three blocks gated on Act 2.1
- align assertions with the new initProject shape
- tests/testthat/test-project-lifecycle.R:
- add saveProject coverage (round-trip, default-path, missing-jsonPath
error, non-Project input)
- add loadProject(exampleProjectPath()) coverage
- tests/testthat/test-project-paths.R:
- add exampleProjectPath coverage
- tests/testthat/test-scenarios-from-pkml.R:
- new file. Cover createScenariosFromPKML happy path, empty input,
paramSheets soft-deprecation
- NEWS.md:
- add Breaking changes, New features, Soft deprecations sections
- _pkgdown.yml:
- surface new exports in the Project workflow section
- move soft-deprecations into a Deprecated section
- R/project.R:
- Project$initialize owns input-shape validation (is.character,
length 1, not NA, nzchar); .loadProjectJson shim removed
- path-field setters return early after write (writing a not-yet-existing
path is normal, e.g. setting an output dir before a run); no more
spurious "File not found" warning on assignment
- path-field getters now use must_work = FALSE. Configurations/,
modelFolder, populationsFolder, dataFolder, the Excel side-car
files all legitimately may be absent (side-cars only land on
exportProjectToExcel; outputFolder only at first run). The existence
check belongs to the code that opens the file, not the field accessor
- R/project-parse.R:
- delete .loadProjectJson; loadProject calls Project$new directly
- R/project-lifecycle.R:
- loadProject is the single user entry point
- R/project-to-json.R:
- rewire doc comments away from the deleted .loadProjectJson
- R/example.R:
- delete; exampleDirectory() was unused outside dead helpers
- inst/extdata/examples/TestProject/:
- relocate to tests/testthat/data/TestProjectExcel/. inst/ ships user-
facing assets, not test fixtures
- tests/data/ → tests/testthat/data/:
- 13 standalone files (pkml, xlsx, csv, RDS) move to the canonical
testthat data root, alongside TestProject/ and TestProjectExcel/
- tests/testthat/helpers.R:
- drop dead helpers: testProjectConfigurationPath, testProjectConfiguration,
local_test_project, testDataDirectory, example_project_json_path,
loadTestSimulation, getSimulationFilePath, testProjectPath
- add testProject() and exampleProject() shorthand helpers
- add testProjectExcelPath() and testProjectExcelConfigurationsPath()
pointing at the relocated Excel fixture
- getTestDataFilePath rewires to testthat::test_path("data", ...)
- tests/testthat/:
- migrate esqlabsR:::.loadProjectJson(path) → loadProject(path)
- migrate loadProject(testProjectPath()) → testProject()
- migrate loadProject(exampleProjectPath()) → exampleProject()
- point test-populations.R at testProjectExcelConfigurationsPath
- replace hard-coded "../data/..." paths with getTestDataFilePath()
- drop dead local example_project_json_path() in test-scenario.R
- tests/testthat/test-project-lifecycle.R, test-scenarios-from-pkml.R:
- stop snapshotting validateIsOfType errors; their formatted message
embeds the caller name via sys.calls() walk, which differs across
devtools::test(), the r-btw MCP server, and CI. Use expect_error()
instead
Excel reading/writing outside the Excel <-> JSON interop is removed. Users wanting to mix Excel into the JSON-first flow should go through importProjectFromExcel() / exportProjectToExcel() on a Project. Removed (no R-side callers; only tests referenced them): - loadObservedDataFromExcel(), loadObservedDataFromPKML() - readPopulationCharacteristicsFromXLS() - readIndividualCharacteristicsFromXLS() - writeIndividualToXLS() - writeParameterStructureToXLS() - exportParametersToXLS() - .readIndividualParameterSetsFromXLS(), .readOntongeniesFromXLS(), .splitCommaString() (internal helpers, dead) - .extractDataCombinedNamesForPlots(), .readPlotConfigurations(), .validateExportConfigurationsFromExcel() (orphans of the deleted Excel plotters) - messages$errorWrongPopulationName, errorWrongIndividualId, errorIndividualParameterSetNotFound, missingOutputFileName, missingPlotGrids, invalidPlotGridNames (orphan helpers) Retained as thin deprecation wrappers (match json-as-primary-input-v2 end-state): - createDataCombinedFromExcel(...) -> createDataCombined(...) - createPlotsFromExcel(...) -> createPlots(...) Retained as standalone Excel helpers that do not take a project-config shape (match end-state): - extendPopulationFromXLS(population, XLSpath, sheet) builds on a Population object directly - readParametersFromXLS() reads bundled SpeciesParameters.xlsx and is still called from the JSON runtime tests/testthat/: - delete test-individuals.R (all tests exercised the removed readers) - trim test-populations.R (drop readPopulationCharacteristicsFromXLS and ontogeny tests; keep extendPopulationFromXLS coverage) - trim test-parameters.R (drop writeParameterStructureToXLS / exportParametersToXLS tests; keep readParametersFromXLS coverage) - drop the skipped loadObservedDataFromExcel test in test-utilities-data.R - drop the skipped createDataCombinedFromExcel test in test-data-combined.R NEWS.md, _pkgdown.yml: reflect removals and surviving deprecation wrappers. tests/testthat/setup.R: clarify why we default lifecycle_verbosity to "quiet".
- R/: - rename utilities-data.R, utilities-file.R, utilities-parallel.R, utilities-quantity.R, utilities-sensitivity-calculation.R, utilities-simulation.R, utilities.R, sensivitity-time-profiles.R (also fixes typo) to per-domain names - fold project-parse.R parse helpers into the section files (individuals.R, populations.R, applications.R, scenarios.R, plots.R, parameter-identification.R) - fold validation-result.R and validation-utils.R into validation.R - fold enum.R into utils.R; move .validateParametersStructure from error-checks.R to parameters.R and .validateClassHasField to plots.R; remove error-checks.R - move geomean and geosd into data-utils.R and .parseSimulationTimeIntervals into utils.R - move applyIndividualParameters from individuals.R to simulation.R - split scenarios.R: saveScenarioResults and loadScenarioResults move to scenario-results.R - split plots.R into plots.R (section parse/validate/serialize/mutation), create-plots.R (createPlots + Excel helpers), plots-utils.R (palette, configuration constructors, override and overlay helpers) - move plot serializers (.plotsToJson, .dataCombinedToNestedJson, .dataFrameToListOfLists) from project-to-json.R to plots.R - delete esqlabs-plot-export-configuration.R; remove createEsqlabsExportConfiguration. ExportConfiguration is no longer supported - parameter-identification.R: replace @method + @rawNamespace S3method with @exportS3Method on print.PIParameter / print.PIOutputMapping / print.PITask so document() no longer warns - tests/testthat/: - rename test files to mirror new sources: test-data-utils.R, test-file-utils.R, test-parallel.R, test-sensitivity-calculation-utils.R, test-simulation.R, test-scenario-from-pkml.R, test-utils.R - rename _snaps/utilities-sensitivity-calculation/ to _snaps/sensitivity-calculation-utils/ - distribute test-project-parse.R, test-project-paths.R, test-project-initialization.R, test-project-mutation-api.R into matching test files (test-project.R, test-project-lifecycle.R, test-output-paths.R, test-individuals.R, test-parameters.R, test-scenarios.R, test-plots.R) following the rule one source file maps to one test file - new files: test-applications.R, test-create-plots.R, test-individuals.R, test-output-paths.R, test-plots-utils.R, test-scenario-results.R - fold test-enum.R into test-utils.R, test-utilities-quantity.R into test-utils.R, test-project-initialization.R into test-project-lifecycle.R - delete test-esqlabs-plot-export-configuration.R and the createEsqlabsExportConfiguration test in test-plots.R - NAMESPACE: regenerated; ExportConfiguration and createEsqlabsExportConfiguration removed; S3method() entries for print.PI* now via @exportS3Method - NEWS.md: extend the internal reorganisation bullet to cover the new renames, foldings, and the plots split; add a Breaking changes entry for the ExportConfiguration removal - _pkgdown.yml: drop the Inherited classes section - man/: regenerate after the renames and removals
Convert the .createPlotGridsFromDataFrames doc to a real roxygen block tagged @nord so the function stays undocumented; drop the stray man page that the leftover @noRd-as-plain-comment generated. - R/create-plots.R: - remove the orphan "Update Plot Configuration with Overrides" roxygen block - rewrite the plain-comment doc above .createPlotGridsFromDataFrames as a #' block with @nord - man/dot-createPlotGridsFromDataFrames.Rd: - delete the wrongly generated topic
- DESCRIPTION: - bump ospsuite Depends floor from 12.2.0 to 12.4.2 (the released version)
- R/messages.R: - rename section headers to match new filenames (populations, data-utils, plots, quantities) - R/project-excel.R: - drop apologetic TODO about messages$restoredProject placeholder - R/scenario-execution.R: - remove comment pointing to deleted R/utilities-scenarios.R wrapper - R/scenario-results.R: - rewrite saveScenarioResults() example to use loadProject() and runScenarios(project) - rewrite loadScenarioResults() example, fix saveResults typo and scnarioNames misspelling - update projectConfiguration @param and outputFolder default-path docstring to refer to Project - man/saveScenarioResults.Rd, man/loadScenarioResults.Rd: - regenerate
+ enable previously skiped tests on macos + update snapshots
- R/scenario.R:
- add read-only asList active binding returning a plain list of field
values via introspection, parallel to Project$asList; useful for
programmatic inspection and snapshot-testing the full object shape
in one assertion
- man/Scenario.Rd:
- regenerate
The current vignettes target the legacy Excel-based API and cannot run against the JSON-first workflow this chapter establishes. Make them inert (eval = FALSE) so R CMD check and pkgdown still parse them without executing broken setup. Chapter 9 will rewrite the prose and re-enable execution. - vignettes/advanced.Rmd, design-scenarios.Rmd, esqlabsR.Rmd, pi-workflow.Rmd, plot-results.Rmd, project-structure.Rmd, sensitivity.Rmd, sensitivity-plots.Rmd, workflow-overview.Rmd: - add eval = FALSE to knitr::opts_chunk\$set with a TODO(#908) marker - vignettes/run-simulations.Rmd: - same eval = FALSE addition; replace the broken createProjectConfiguration(ignoreVersionCheck = ...) setup call with a TODO comment - vignettes/pi-workflow.Rmd: - replace devtools::load_all() in the setup chunk with library() and a TODO comment
The file-level opts_chunk$set(eval = FALSE) was overridden by a single eval = TRUE chunk that still called the legacy createProjectConfiguration(), which broke R CMD check on CI even though the rest of the vignettes are inert. Flip it to eval = FALSE with a TODO(#908) marker so vignette building stays green until chapter 9 rewrites the prose. - vignettes/project-structure.Rmd: - flip eval = TRUE to eval = FALSE on the createProjectConfiguration chunk - add TODO(#908) note alongside the deferred chapter 9 rewrite
Bundle of small, mechanical clean-ups identified during the chapter 8
review: tighten type-stability, drop a runtime-reflection contract,
consolidate global-variables registries, and stop mutating session
state from the package's .onLoad.
- R/project.R:
- swap `must_work == TRUE` for `isTRUE(must_work)` in path resolution
- R/sensitivity-time-profiles.R:
- rewrite `.isConvertableUnit` as a value-returning tryCatch (no `<<-`)
- R/globals.R, R/zzz.R:
- merge the two `utils::globalVariables` blocks into globals.R
- sort the merged vector alphabetically
- R/zzz.R, tests/testthat/setup.R:
- move `colorPalette` assignment into .onLoad (no top-level side effect)
- move `_R_CHECK_LENGTH_1_CONDITION_` env var from .onLoad to test
setup via withr::local_envvar so it does not mutate the user session
- R/scenario.R, tests/testthat/test-scenario.R,
tests/testthat/_snaps/scenario.md:
- replace `ls(self)` reflection in `Scenario$asList` with an explicit
`.scenarioFieldNames` constant
- add field-list and read-only tests
- R/data-utils.R, R/file-utils.R, R/scenario-from-pkml.R,
R/simulation.R, R/utils.R, R/validation.R:
- swap `sapply()` for `vapply()` (or `lapply()` where simplification
was already opted out via `simplify = FALSE`)
- R/create-plots.R, R/data-combined.R, R/observed-data.R:
- drop `ospsuite.utils::` prefix on `validateIsOfType`; the package
already imports ospsuite.utils, so bare calls match the dominant
style in the codebase
Every call site already passed `must_work = FALSE`, so the existence-check
branch and the per-instance `.warned_paths` dedupe cache were both
unreachable. Removing them simplifies the path resolver and eliminates the
unbounded-growth concern flagged in the cleanup audit.
- R/project.R:
- drop `must_work` parameter from `.clean_path` and the matching
`must_work = FALSE` argument from every active-binding call site
- delete the unreachable `fs::file_exists` / dedupe block
- delete the `.warned_paths = character()` private field
- tests/testthat/test-project.R:
- drop the now-invalid `must_work = FALSE` argument from the
.clean_path env-var test
`.safe_validate` had no callers and its only collaborator, `.categorize_message`, was a regex-based heuristic that only fired from that dead path. Every validation adapter passes a literal category at the call site, so the regex was redundant the moment the convention landed. - R/validation.R: - delete `.safe_validate` and `.categorize_message` - man/dot-categorize_message.Rd, man/dot-safe_validate.Rd: - delete the now-orphan Rd files
Before: the blanket tryCatch around build + run swallowed every error
into `warningPIOptimizationFailed`, including user typos (parameter
path not found, unknown output, missing observed-data id). The task
appeared "skipped" with no actionable signal.
After: `.createSinglePITask` runs outside the tryCatch; only
`runtime$run()` is wrapped, so config errors hard-fail and the user
sees them immediately. The soft-fail entry now keeps the built runtime
on `task` (was `NULL`) so callers can still introspect the optimisation
setup.
Snapshot fix bundled in: the createPITasks / runPI deprecation snapshots
still said `6.1.0` after the in-source version was consolidated to
`6.0.0`. Update the snapshot to match the source.
- R/parameter-identification.R:
- move `.createSinglePITask` call out of the tryCatch
- tryCatch now wraps only `runtime$run()` and carries the runtime in
the soft-fail entry
- update `runPI` `@returns` and add a paragraph distinguishing build
vs optimisation failures
- R/messages.R:
- tighten `warningPIOptimizationFailed` wording to say "optimisation
failed" so the warning is clearly about the optimiser, not the
build phase
- NEWS.md:
- extend the runPI bullet with the new build-error contract
- man/runPI.Rd:
- regenerated
- tests/testthat/test-parameter-identification.R:
- add `runPI(project) hard-fails when the build phase errors`
(mocks `.createSinglePITask` to throw, expects an error)
- add `runPI(project) soft-fails when the optimisation phase errors`
(mocks the runtime to throw inside `run()`, expects a warning and
the built runtime still on `task`)
- tests/testthat/_snaps/parameter-identification.md:
- update deprecation version snapshots from 6.1.0 to 6.0.0 to match
the consolidated lifecycle calls
Every `project$addX(...)` / `project$removeX(...)` method on the Project
R6 class was a thin forwarder to the standalone function of the same
name (`addX(project, ...)`). Two parallel public APIs for the same
operation are a maintenance liability and confuse users about which
form is canonical.
Pick the free-function form as canonical (it is what every existing
test exercises) and drop the wrappers outright. The Project class
keeps only its real methods: lifecycle helpers (`.markModified`,
`.markSaved`, `.markValidated`, `.getFilePathsData`), `sync()`, and
`print()`.
- R/project.R:
- delete all `addX` / `removeX` / `setX` R6 method wrappers from the
public block (442 lines)
- man/Project.Rd:
- regenerated; only lifecycle, sync, print, and clone remain
- tests/testthat/test-project.R:
- drop the now-obsolete `project$addX delegates to the standalone addX`
test (free-function coverage already lives in test-individuals.R,
test-populations.R, test-applications.R, test-output-paths.R, etc.)
- NEWS.md:
- bullet under "Breaking changes" documenting the removal and pointing
users to the free-function form
Migrate every `stop(messages$...)` / `warning(messages$...)` / inline
`stop("...")` site introduced or rewritten by the chapter-1-to-8 chain
to `cli::cli_abort()` / `cli::cli_warn()`. The legacy code that already
existed on `v6` (sensitivity-*, create-plots Excel helpers,
plots-utils, data-utils, utils) keeps its existing call style; that
sweep is tracked as a follow-up so the diff stays scoped.
The `messages$` registry entries are untouched: every chapter-chain
message already returns a `cliFormat()`-rendered string, which cli_abort
treats as inline content. The only visible behavioural change is the
condition class (`rlang_error` / `rlang_warning` instead of bare R
conditions) and cli's terminal-width line wrapping, which forced minor
snapshot updates.
- R/parameter-identification.R, R/parameters.R, R/populations.R,
R/data-combined.R, R/project-lifecycle.R, R/project-excel.R,
R/scenario-from-pkml.R, R/esqlabs-env.R:
- swap `stop(messages$...)` for `cli::cli_abort(messages$...)`
- swap `warning(messages$...)` for `cli::cli_warn(messages$...)`
- R/project.R, R/project-to-json.R:
- same sweep, plus convert the inline `stop("...", call. = FALSE)`
sites (read-only field guards, parent-dir check, schema-version
check, Project type guard) to `cli::cli_abort` with cli templating
- R/scenarios.R:
- convert two inline string-concat `stop(...)` blocks in
`.parseScenarios` to `cli::cli_abort` with cli templating
- tests/testthat/test-data-combined.R, test-parameters.R,
test-project-to-json.R:
- relax `expect_error(..., regexp = messages$x(...))` matches to a
distinctive phrase. The pre-rendered messages contained literal
newlines that don't survive cli's wrapping pass; the phrase match
keeps the assertion meaningful without coupling to cli's line breaks
- tests/testthat/_snaps/parameter-identification.md,
_snaps/populations.md:
- regenerate to reflect cli's wrapping (`testthat::snapshot_accept`).
Message text is unchanged; only line-break positions differ
- NEWS.md:
- bullet under `## Internal` noting the chapter-chain converges on
cli and pointing at the follow-up for legacy code
- R/project-excel.R: - flatten outputPaths with unlist() when building the OutputPaths sheet - tests/testthat/test-project-excel.R: - cover OutputPaths sheet columns on export - cover Project round-trip through Excel preserving outputPaths
- R/project-excel.R: - drop unused schemaVersion parameter from the five .parseExcel* helpers and their call sites in importProjectFromExcel() - remove the dead esqlabsRVersion read (overwritten before the JSON is written) - drop unused ignoreVersionCheck parameter from projectStatus() - delete the stale TODO comment above the restoredProjectConfiguration message - R/project.R: - simplify the redundant !missing guard in Project$initialize() - man/Project.Rd, man/projectStatus.Rd: - regenerate
Disable evaluation of vignettes that still call the removed Excel-first API so vignette re-building no longer fails R CMD check, pending the chapter 9 rewrite. Convert two caller-sensitive error snapshots to expect_error so they no longer pin the volatile calling frame. - vignettes/advanced.Rmd, vignettes/design-scenarios.Rmd, vignettes/esqlabsR.Rmd, vignettes/pi-workflow.Rmd, vignettes/plot-results.Rmd, vignettes/project-structure.Rmd, vignettes/run-simulations.Rmd: - add global eval = FALSE to opts_chunk$set with a TODO(#908) marker - R/scenario-from-pkml.R: - use the string form validateIsOfType(project, "Project") to match the rest of the package - tests/testthat/test-project-lifecycle.R: - assert the non-Project input error with expect_error(..., "Project") - tests/testthat/test-scenarios-from-pkml.R: - assert the non-Project input error with expect_error(..., "Project") - tests/testthat/_snaps/project-lifecycle.md: - drop the now-unused non-Project error snapshot - tests/testthat/_snaps/scenarios-from-pkml.md: - remove the file; its only snapshot is now an expect_error
…ain-reorg # Conflicts: # R/project.R
Resolve the #1037 (chapter 7) PI-rework conflict with chapter 8a's parallel copy. Both branches landed an identical R/parameter-identification.R; 8a's design (read-only Project, single-arg initialize, .read_json, sync()) is the one the stack builds on (8b inherits it), so: - R/project.R, R/project-parse.R: keep 8a (ours); PI parsing already wired via Project$.read_json() calling .parsePITasks(). - tests/testthat/test-parameter-identification.R: take #1037 (theirs), a strict superset (+7 .createSinglePITask tests); passes against 8a's code. - NAMESPACE/man: regenerated via devtools::document().
- R/project.R: - drop the unreachable must_work == TRUE file-not-found warning branch - drop the now-unused private$.warned_paths field - drop the vestigial must_work parameter from the signature and all 11 path-field getters - tests/testthat/test-project.R: - drop the must_work argument from the direct .clean_path() call
# Conflicts: # R/project.R
- R/esqlabs-env.R: - getEsqlabsRSetting(): rlang::abort() -> cli::cli_abort(), the lone site the chapter-chain sweep missed - R/observed-data.R: - source-types comment: project$addObservedData() -> addObservedData(project) (R6 method removed in this PR) - tests/testthat/test-esqlabsr-settings.R: - switch the raw-message expect_error() to expect_snapshot(error = TRUE); cli re-wraps the message at terminal width so the old pattern no longer matched - tests/testthat/_snaps/esqlabsr-settings.md: new snapshot
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## json-based-project #1069 +/- ##
======================================================
- Coverage 81.59% 76.78% -4.81%
======================================================
Files 48 32 -16
Lines 10119 7113 -3006
======================================================
- Hits 8257 5462 -2795
+ Misses 1862 1651 -211 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Base automatically changed from
excel-deprecation-and-reorg
to
json-based-project
June 19, 2026 15:12
This was referenced Jun 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add v6 project-snapshot JSON Schema (proposed clean variant) — RFC
What
Adds a proposed, normalised Draft 2020-12 schema under
inst/schema/proposed-clean/,as an alternative to the faithful schema in the companion PR. Same modelling information,
cleaner encoding. This is an RFC — not for merge until accepted.
Why (summary; full argument in
RATIONALE.md)The faithful schema faithfully encodes three transitional warts from the current v6 output.
This PR proposes fixing them now, while they are cheap, before ESQdb and Ralf depend on them:
id(today v6mixes arrays-with-id and name-keyed maps). Better for ESQdb rows, more reliable for LLM
generation, and JSON Schema validates array items far better than map values.
simulationTime,xValuesLimits/yValuesLimits,plotIDs,proteinOntogeniesfrom delimited strings to realarrays/objects, so malformed values fail at validation instead of silently at runtime.
idfield across object types instead ofname/individualId/populationId/id.None of these change the information carried — they are migrations, carried by a
schemaVersionbump.Decision owner
@Felixmil — accept/reject as a whole or point by point.
RATIONALE.mdis written for this.Validation
The proposed schema validates a normalised form of
inst/extdata/projects/Example/Project.json(transform included in the PR discussion). All files are valid Draft 2020-12.
Relationship to companion PR
Independent branch, separate directory (
inst/schema/proposed-clean/), no file conflictwith the faithful PR — either can merge first.