Closed schuemie closed 4 years ago
If we agree with this pattern, I suggest we delete:
getCohortDefinitionExpression
(because you could getCohortDefinition
and then save as JSON. Maybe we do need a dedicated function for that operation)getCohortDefinitionName
(because you could getCohortDefinition
and just get the name from the R object)getConceptSetsAndConceptsFromCohort
(getCohortDefinition
, then get the concept sets from the R object and call resolveConceptSet
)@alondhe , @gowthamrao , what do you think?
@schuemie If we do this, i think we may need to add some functionality to get concept set expression from within cohort definitions.
But we already have that functionality in R:
cohortDefinition <- getCohortDefinition(cohortId = 14840, baseUrl = baseUrl)
conceptSet <- cohortDefinition$expression$ConceptSets[[1]]
conceptSet$name
# [1] "[OHDSI Cov19] Hepatitis C protease inhibitors"
conceptIds <- resolveConceptSet(conceptSet = conceptSet, baseUrl = baseUrl)
head(conceptIds)
# [1] 793843 793858 793860 793861 793862 793863
We just need to explain that to users in a vignette.
Sounds good - agree we have an alternative. If we agree, we can merge these change to develop branch. I will add a comment to the vignette issue.
@schuemie
Functions that go from WebApi to (comples) R objects that contain no JSON text. These objects can be inspected in R, for example to get the name of a concept or concept se
did you mean "Functions that go from WebApi to (comples) R objects should contain no JSON text" as described here
There are functions for extracting (complex) R objects from WebAPI. These R objects no longer contain any JSON, but are full R object representations of the JSON, and can therefore be easily used in R, for example cohortId <- estimation$cohortDefinitions[[1]]$id
https://github.com/OHDSI/ROhdsiWebApi/pull/32#issuecomment-603618970
This pattern has been implemented starting v0.1.0
More discussion on design decision here
https://github.com/OHDSI/ROhdsiWebApi/pull/95#issuecomment-631398515
Another generic design decision:
In general, ROhdsiWebApi will support singular calls on API end points. i.e. functions wont be built to loop through the same end points and return appended objects.
e.g. we will have a function getCohortDefinition (singular), but we wont have a function called getCohortDefinitions (plural) that loops through multiple cohort definitions.
Similarly, we will have a function to invoke a cohort generation, but we wont have invoke cohort generation (s) that will loop through combination of source keys and cohort ids. The looping is expected to be done by end-user using code like
lapply(cohortIds, invokeCohortGeneration)
Continuing the discussion from here
So the proposal is that the main design pattern for this package is:
This would reduce the growth of functions, which currently aim to go from WebApi directly to the desired output.
@gowthamrao: Yes, the idea would be the R objects are faithful representations of the JSON objects, so conversion back to JSON should be trivial. The new
resolveConceptSet
function I created follows this pattern.Could you explain what you mean when you say you "have struggled with using R-objects as storage"? Saving them as RDS objects works great, and if we want more portable objects, we can always saved them as JSON (at the time of saving). In fact, we use JSON to store R objects such as the CohortMethod analysis settings all the time.
Could you describe uses cases where we need to pull large data objects from WebAPI? It helps to have specific scenarios in mind when discussing how we would approach them.