Change filter parameter default to TRUE

[DeclanHodges]

Proposed change of approach:
Currently, we use NULL as the default argument in our functions when we want to make filtering optional. This requires us to call filter_if() within our functions, which only filters the input dataset if the provided filter condition is not NULL.

However, if we set the default argument to TRUE, we would no longer require the use of filter_if() and could instead just use filter().

These files use filter_if() and so would be updated if this change goes ahead:

• derive_extreme_records.R
• derive_joined.R
• derive_merged.R
• derive_param_computed.R
• derive_param_exposure.R
• derive_param_tte.R
• derive_var_dthcaus.R
• derive_var_extreme_date.R
• derive_vars_last_dose.R
• derive_vars_transposed.R
• get_summary_records.R

See original discussion in #1900 (comment)

[zdz2101]

@pharmaverse/admiral I don't think this change is absolutely necessary, but if we were to implement, would we want to stay consistent with this swap and replace default values for arguments like filter and filter_join into TRUE as well? And such, replace all instances of filter_if() with filter(!!<filter, filter_add, filter_join>) ?

[bms63]

Hi @pharmaverse/admiral and @zdz2101 are moving forward with this update? need more discussion? or not going to be pursue this anymore?

[bundfussr]

I would not implement it now but investigate and discuss it in the clean-up meetings. We should be sure that it works for all cases and that the behaviour is consistent from the users point of view. I.e., we need to check all filter* arguments even those where the function does not call filter_if(). We should also consider similar arguments like condition or first_cond.

[manciniedoardo]

My two cents: this functionality may be a “nice to have”, but I don’t think it’s necessary to devote lots of time to implementing it. I would propose closing this issue until we have a real need to do this.

[bms63]

I'm going to move to the Discussion Re-Work Part 3 Thread.