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 insightsengineering/teal.modules.clinical#1230 (comment)
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
insightsengineering/teal.modules.clinical#1230 (comment)
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.
Originally posted by @donyunardi in insightsengineering/teal.modules.clinical#1230 (comment)
_Originally posted by @pawelru in
insightsengineering/teal.modules.clinical#1230 (comment)
Summary
We want to promote the use of the
withinfunction when buildingteal_datawith the CDISC data format. As mentioned in the discussion above, the purpose ofcdisc_datais lost when usingwithin, since users will still need to set upjoin_keysmanually.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.