-
-
Notifications
You must be signed in to change notification settings - Fork 9
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_keysfor the datasets at creation. However, if we're promoting the use ofwithinto avoid code repetition (which I completely agree), users will still need to set up thejoin_keysthemselves, 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 thedefault_cdisc_join_keyshelper 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 whereascdisc_data()requires all the information on that object init. As we are moving towards the use ofwithin()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()oras_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.