Aggregate widget should be more consistent with Group-by widget - Githubissues

biolab / orange3-timeseries

🍊 :chart_with_upwards_trend: Orange add-on for analyzing, visualizing, manipulating, and forecasting time series data.

Other

62 stars 40 forks source link

Aggregate widget should be more consistent with Group-by widget #192

Closed wvdvegte closed 2 years ago

wvdvegte commented 2 years ago

Timeseries version

0.3.12

Orange version

3.32

Expected behavior

Since the functionality of Aggregate is very similar to Group-by, it should offer the same possibilities and the same outputs

Actual behavior

In Group-by, it is possible to generate multiple aggregations per variable. For instance new columns with the count and the mean of the same variable. In Aggregate, only one aggregation per variable is allowed. To produce multiple aggregations, outputs from multiple Aggregate widgets have to be combined.
For categorical variables, several aggregations are possible in Group-by (but Mode is not available. In aggregate, only Mode is available, although others, such as Count, would certainly make sense.
In the output of Group-by, the type of aggregation is added to the name of the source variable in the column names (e.g. "distance - Mean"). In the output of Aggregate, it's just the name of the source variable ("Distance")

Steps to reproduce the behavior

Apply Aggregate and Group-by to time-stamped data with variables of different types, and check out the options that are offered

Additional info (worksheets, data, screenshots, ...)

janezd commented 2 years ago

In #197, widgets Moving Transform and Aggregate, which share a lot of functionality, will be merged into a single widget, which will be similar to Group by.

wvdvegte commented 1 year ago

In Timeseries 0.5.1, the suggested aggregation functionality has indeed been included in Moving Transform. But why is Aggregate still there? For backward compatibility? If I want to aggregate over time, my first guess would be to use Aggregate - which has limited functionality. Suggestion: rename Moving Transform to Moving Transform & Aggregate and/or make it clear somewhere that Aggregate is legacy widget (for instance in its dialog box). By the way, the help link in Aggregate's dialog box is currently not working" "There is no documentation for this widget."

janezd commented 1 year ago

Aggregate has been removed in https://github.com/biolab/orange3-timeseries/pull/205. It should no longer appear.

I just asked @markotoplak and we don't understand how could it remain in your system. Which OS do you have and how do you install Orange and add-ons?

wvdvegte commented 1 year ago

@janezd , @markotoplak , I'm using Orange 3.32.0 and Timeseries 3.5.1. I double-checked and Aggregate is still there. Edit - my system is Mac OS 12.6 on Silicon. I installed Orange from DMG download, and add-ons through Options -> Add-ons

ajdapretnar commented 1 year ago

I can confirm Aggregate is absent from my DMG install. There could be some registry issue though...

wvdvegte commented 1 year ago

Could it be because, after installing Timeseries 0.5.1 (not 3.5.1!), I loaded an older workflow that still contained the Aggregate widget? What happens, anyway, if you load a workflow containing an abolished widget?

ajdapretnar commented 1 year ago

What happens, anyway, if you load a workflow containing an abolished widget?

Widgets have legacy settings, so each widget would know which other widget it replaces. From what I see, Moving Transform already does that in L96: replaces = ["orangecontrib.timeseries.widgets.owaggregate.OWAggregate"]

wvdvegte commented 1 year ago

One question and one suggestion:

What can I do to get rid of the Aggregate widget and/or how can I find out if it's there due to a "registry issue" (and how to resolve it)?
I propose to rename the Moving Transform, now that it also covers aggregation. Aggregation is not really "moving", so the name is misleading. Perhaps the current overall functionality is actually closer to integration (as opposed to the Difference widget), so perhaps it could be renamed to Integrate or Integral, using an integral symbol as its icon ...

ajdapretnar commented 1 year ago

Ad 1: No idea. Programmatically I know how, but you said you installed the add-on via DMG, so 🤷‍♀️ from my side. Ad 2: We have discussed this before and aggregation is kind of a moving transform, because it moves along the timeline and aggregates chunks. Typing "agg" in the menu search will propose Moving Transform at the top, because the widget includes "aggregate" as a keyword. Your suggestion is simply too long. At best "Aggregate and Transform", but others would have to confirm the choice.

wvdvegte commented 1 year ago

Ad 1: could uninstalling and reinstalling everything possibly help? Ad 2: but "Integrate" isn't too long ... Anyway, I see your point. But then it's actually a sub-option of "Consecutive blocks", with varying block width, depending on the number of rows per unit of time ...