Remove required from yamls and associated functions
#1951
Conversation
| @@ -81,7 +81,7 @@ CheckSpec <- function(lData, lSpec) { | |||
| } | |||
| # check that required exist in data, if _all required is not specified | |||
| if (!isTRUE(lSpec[[strDataFrame]]$`_all`$required)) { | |||
There was a problem hiding this comment.
Keeping required on _all?
There was a problem hiding this comment.
This makese sense to me - noticed a few unquoted data types. I'm wondering if the _all$required scenario needs to be reconsidered. It basically just tells ApplySpec to keep everything, but the specification is a little esoteric. That said, if it works it works.
| Mapped_STUDY: | ||
| _all: | ||
| required: true |
There was a problem hiding this comment.
were these meant to be removed? And does it need to be done on:
3_reporting/Bounds.yaml
4_modules/report_kri_country.yaml
4_modules/report_kri_site.yaml
inst/examples/2_AdverseEventWorkflow.R
?
There was a problem hiding this comment.
we went back and forth, but for the moment, we are leaving the _all specs as is, and will handle them later via #1952
There was a problem hiding this comment.
Understood, added a commit to remove a couple lines of inst/examples/2_AdverseEventWorkflow.R where it was last stragglers for not-_all: required
There was a problem hiding this comment.
oh nice- thanks for catching that!
Overview
Remove the required field from yaml specs and assume that all columns specified in the spec are required. Adjusts
checkSpec()andCombineSpec()appropriately.Also changes experimental flags to stable for FlagOverTime and report table creation functions
Test Notes/Sample Code
Connected Issues
requiredfield from specs andcheckSpec()/CombineSpec()/Ingest()#1936