Maarten van Kessel
Maarten van Kessel
@azimov I took the liberty to trace down the path `callingArgsJson` is used for. From what I can gather it follows this path: 1. `executeDiagnostics()` RunDiagnostics.R a. [L217-L232](https://github.com/OHDSI/CohortDiagnostics/blob/main/R/RunDiagnostics.R#L217-L232) b. [L962-L1014](https://github.com/OHDSI/CohortDiagnostics/blob/main/R/RunDiagnostics.R#L962-L1014),...
I think splitting out the metadata in two different classes is a good approach. I think for inheritance sake, it would be best for the "data-loading meta data" to inherit...
> Should `MetaDataPS` inherit from `MetaDataLoading`, instead of the other way around? This would follow the order in which they are created: `getDbCohortMethodData()` would create `MetaDataLoading`, and `createPs()` would extend...
I've added the following functions in the DataLoadingSaving.R: 1. checkParameters [L288:L330](https://github.com/OHDSI/CohortMethod/blob/is134_checkParams/R/DataLoadingSaving.R#L288-L330) 2. downSample [L332:L379](https://github.com/OHDSI/CohortMethod/blob/is134_checkParams/R/DataLoadingSaving.R#L332-L379) 3. removeDupes (working title) [L381:445](https://github.com/OHDSI/CohortMethod/blob/is134_checkParams/R/DataLoadingSaving.R#L381-L445) 4. getPresampleCount [L447:L457](https://github.com/OHDSI/CohortMethod/blob/is134_checkParams/R/DataLoadingSaving.R#L447-L457) The way `getDbCohortMethodData` is currently setup makes it...
> I must admit I'm not sure this is really an improvement. Functions are normally a good way to reduce complexity because the only way a function can interact with...
> (Although ideally I would explicitly limit its input to the `getDbCohortMethodData()` argument, for example using `match.call()`). > Cool! I will update the functionality to use `match.call` instead of an...
> I think I actually prefer option 4: > > 4. Keep the parameter checks in the main function. > > Let me try to explain why: > > *...
Hi @schuemie as per [this](https://github.com/OHDSI/CohortMethod/pull/136#issuecomment-1507154687) comment: >Looks great! As discussed, further refactoring would probably require turning the meta-data into some nice object that can be passed around by reference. But...
> Thanks! Could you help me understand your overall design here? You have > > * a `getDbCohortMethodData()` function > * a `CohortDbInterface` object (R6) > * a `CohortMethodDataR6` object...