cdisc-org / analysis-results-standard

This repository will be where all the results for the Analysis Results Standard will be delivered.
MIT License
50 stars 10 forks source link

TypeScript type errors - inlining of whereClauses in subtypes #207

Open rc-oxpop opened 1 year ago

rc-oxpop commented 1 year ago

Hi! As part of the hackathon I'd like to play around with this model in TypeScript. LinkML has a generator for TypeScript types and interfaces:

gen-typescript .\model\ars_ldm.yaml > model/ars_ldm.ts

but the resulting code (via TypeScript Playground) has a few type mismatch errors.

The reason for this seems to be that in the generated interface WhereClauseCompoundExpression, the whereClauses property has been inlined (and hence has type WhereClause[] in the output TS), whereas in CompoundSetExpression and CompoundGroupExpression, it's not inlined, and so the whereClauses properties there have types which are aliases of string[] (i.e. they're lists of ids).

I think this is because in CompoundSetExpression and CompoundGroupExpression, the whereClauses slot usage is overridden to have inlined: false, whereas in general, where_clauses is inlined (necessarily, since WhereClause has no identifier). Setting the whereClauses slots for CompoundSetExpression and CompoundGroupExpression to be inlined as lists fixes the type errors in the generated TypeScript.

Would inlining the whereClauses slots for those two classes cause problems anywhere else? Or is there a better fix somewhere? I'm happy to raise a PR to fix if you think appropriate.

ASL-rmarshall commented 1 year ago

Hi @rc-oxpop, Thanks for raising this issue. It's interesting to hear that you're playing around with TypeScript - this is a new use case and we haven't tried using the TypeScript generator before.

Based on one of the original expectations for the way in which analysis sets and groupings were to be defined (with compound expressions being defined as combinations where clauses that have defined been defined in other identified analysis sets or groups), the model currently only expects:

The AnalysisSet, AnalysisGroup and DataGroup classes are all defined as is_a WhereClause, so it's their id values that are expected when inlined: false for whereClauses (which was deliberate).

Having said that, I did run into a similar issue when I was trying to create some example groupings with non-inlined compound expressions. Ideally, the model should be able to handle both inlined compound expressions and references to other already-defined (and identified) analysis set/group where clauses. In the LinkML GitHub repo, I have found reference to a construct such as:

any_of:
- inlined: true
- inlined: false

but I haven't (yet) been able to get this to work without error when using the LinkML converter to convert from YAML to JSON format.

Another issue has been raised relating to this (#136) with the expectation that we'll do some additional testing to see if it's feasible to define compound expressions for analysis sets and groupings only with reference to id values. If it's not, we'll then attempt to adjust the model to accommodate both types of specification for whereClauses. However, this issue has not be prioritized (yet) for fixing before the ARS v1.0 release based on the rationale that ARS expects "analysis-ready" ADaM datasets as the input, so the need for compound expressions in analysis sets or groupings should be limited (because you'd usually expect compound expressions already to have been processed for the creation of flag or groupings variables in the ADaM dataset).

Does this give you enough information to continue with your TypeScript development?

(PS - if possible, please raise any additional hackathon-related issues in the hackathon repo. This'll help us with issue curation. Thanks!)