insightsengineering / insightsengineering/teal.data
Simplify usage of `teal_data` for cdisc data format
- Dominant language
- R
- Stars
- 11
- Forks
- 9
- PR merge metrics
- No merged PRs in 30d
Description
> The more I think about this, the more I’m leaning towards deprecating `cdisc_data`.
>
> The only purpose of this wrapper is to guess the `join_keys` for the datasets at creation. However, if we're promoting the use of `within` to avoid code repetition (which I completely agree), users will still need to set up the `join_keys` themselves, which defeats the purpose of the wrapper.
>
> I think it’s much easier if users just work with `teal_data`. If they’re working with ADaM datasets, we can point them to the `default_cdisc_join_keys` helper object if they want to use it.
>
_Originally posted by @donyunardi in https://github.com/insightsengineering/teal.modules.clinical/pull/1230#discussion_r1787943364_
> I have very similar thoughts. The concept of those two differs significantly. `within()` modifies the object in-place and require follow-up updates whereas `cdisc_data()` requires all the information on that object init. As we are moving towards the use of `within()` then I think that that our CDISC convenience function should follow the same paradigm to modify the object based on its current state. When doing that PR I got a little bit tired of setting keys and datanames separately. This feels repetetive and I think it can be easily wrapped up in a single convenience function - e.g. `set_cdisc_attrs()` or `as_cdisc()` so that the full processing can look like this: `teal_data() |> within({...}) |> set_cdisc_attrs()`
_Originally posted by @pawelru in
https://github.com/insightsengineering/teal.modules.clinical/pull/1230#discussion_r1787959135
### Summary
We want to promote the use of the `within` function when building `teal_data` with the CDISC data format. As mentioned in the discussion above, the purpose of `cdisc_data` is lost when using `within`, since users will still need to set up `join_keys` manually.
Let's simplify this by having users only work with `teal_data()`. Any additional setup for CDISC data types can be handled afterward.
Note: We should also be able to avoid requiring users to set up datanames once we're finished with [this](https://github.com/insightsengineering/teal.data/issues/333).
Contributor guide
Assessment
This issue has not been assessed yet.