insightsengineering / cards

CDISC Analysis Results Data
https://insightsengineering.github.io/cards/
24 stars 0 forks source link

Any more pre-release renaming? #184

Closed ddsjoberg closed 4 months ago

ddsjoberg commented 4 months ago

Some naming conventions were done rather quickly, and could perhaps be a bit shorter/better. I couple come to mind:

  1. In every ARD table, we have columns c('stat_name', 'stat_label', 'statistic', 'statistic_fmt'). Would it be better to call them c('stat_name', 'stat_label', 'stat', 'stat_fmt')?
  2. The 'statistic_fmt_fn' is pretty long. Is it better as simply 'fmt_fn'?
  3. I don't typically mind long, descriptive function names (because autocomplete is great). But these are particularly long: continuous_variable_summary_fns(), categorical_variable_summary_fns(). We could probably name them continuous_summary_fns() and categorical_summary_fns() while keeping the names quite descriptive.

@bzkrouse What do you think? Are there any other names that could be improved before the first release?

bzkrouse commented 4 months ago

@ddsjoberg I like it!! I prefer fmt_fn as it matches the function arg Add to the list:

ddsjoberg commented 4 months ago

@bzkrouse that's a good one too! It's going to be annoying to update all of them, but I think it's the right thing to do 😭

Do you think we should do all of these or just some of?

bzkrouse commented 4 months ago

I agree, I think doing all of them makes sense!

Just one more thought - Does it make sense to go all the way to have the function arguments match the cards output? Right now there are going to be subtle differences: e.g. stat_labels (fun) vs stat_label (output), statistic (fun) vs stat_name/stat (output)