You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
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()
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.
The text was updated successfully, but these errors were encountered:
donyunardi
transferred this issue from insightsengineering/teal.modules.clinical
Oct 4, 2024
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
within
function when buildingteal_data
with the CDISC data format. As mentioned in the discussion above, the purpose ofcdisc_data
is lost when usingwithin
, since users will still need to set upjoin_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.
The text was updated successfully, but these errors were encountered: