Commit 29abdda1 authored by Dorchies David's avatar Dorchies David
Browse files

v1.6.8.20 feat: add .AggregConvertFunTable data.frame

- This public data.frame allows the user to watch and modify the auto-guess operations to perform in SeriesAggreg

Refs #41
Showing with 35 additions and 36 deletions
+35 -36
  • @david.dorchies don't you find it is confusing for the user if getAggregConvertFun() is private? It means that he can't see what is the default values of the ConvertFun argument does: getAggregConvertFun(names(x)[-1])

    Edited by Delaigue Olivier
  • mentioned in issue #77 (closed)

    Toggle commit list
  • You are right. Since it is a default argument of ConvertFun argument of SeriesAggreg it should be documented. But I don't think that this function is useful for the user.

    The only information really useful for the user is the data.frame .AggregConvertFunTable which is already public.

    So I suggest to use an argument ConvertFunTable = .AggregConvertFunTable instead of ConvertFun = .AggregConvertFun(names(TabSeries)[-1]) in SeriesAggreg and to call the private function getAggregConvertFun(ConvertFunTable) in SeriesAggreg.

  • ... And .AggregConvertFunTable should be better documented either in the SeriesAggreg page or in a dedicated page.

  • In this case, why not just keep only the argument ConvertFun = NULL by default and explain in the help page that in this case it uses the table .AggregConvertFunTable. A ConvertFunTable argument will not be very easy to check. Don't you think so?

    Edited by Delaigue Olivier
  • A ConvertFunTable argument will not be very easy to check. Don't you think so?

    I don't think so. It involves only 4 conditions:

    if(!is.data.frame(ConvertFunTable) stop...
    if(names(ConvertFunTable) != c("ConvertFun", "Outputs")) stop...
    if(!all(ConvertFunTable$ConvertFun %in% c("mean", "sum"))) stop...
    if(!is.character(ConvertFunTable$Outputs)) stop

    I still think that exposing ConvertFunTable instead of ConvertFunis a better idea because:

    • it's more complicated for the user to write a ConvertFun function than modifying .AggregConvertFunTable or create is own two column-data.frame
    • now that you've added messages in getAggregConvertFun for unknown object, it should not be overwritten by the user in SeriesAggreg
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment