AuroreAA / ICSClust

What the Package Does in One 'Title Case' Line
GNU General Public License v3.0
0 stars 0 forks source link

Adding the `scale` argument to all `select_crit` functions? #8

Closed AuroreAA closed 1 year ago

AuroreAA commented 1 year ago

I wonder if I should add the scale argument to the functions: normal_crit.ICS, med_crit.ICS, var_crit.ICS and discriminatory_crit.ICS. In fact, I am using the gen_kurtosis method for med_crit.ICS and var_crit.ICS and the question is: should I add the scale argument even though it should not changed the results. Then for the select_plot function should I provide the scale argument and compute it again in this case?

aalfons commented 1 year ago

It seems minimal programming effort, so from a programming perspective I don't see much of a reason not to do it. But I'm not sure if it's useful in practice. Perhaps @klauschN can give an answer on the usefulness?

klauschN commented 1 year ago

In the moment I do not see really the usefulness - but I agree, it does no harm neither and seems not to difficult to add. Maybe it might be safer to add it now...

AuroreAA commented 1 year ago

After discussing with Andreas, we are not sure if it makes sense to add it because it will complexify quite a lot of the functions and it won't be clear for the user how to use it. Would you mind if I never scale the generalized kurtosis values in ICSClust?

klauschN commented 1 year ago

Yes, it is also fine for me to drop scaling there.