statistics: add a sort mode for categorical bar charts

This was a user request: Sort bar charts by height of the bars.
Obviously, this can only work for categorical charts, not for
histograms.

The UI is a break from the old concept: the sorting is chosen
based on the chart, whereas for the rest of the features, the
viable charts are presented based on the binning, etc.

I found it confusing to have the possible charts be selected
based on sorting. I.e. if a non-bin sort mode is selected,
the histogram charts disappear. On the flip side, this would
be more consistent. We can change it later.

For value-based bar charts, there are three sort modes: by
bin, by count (i.e. number of dives in that bar) and by
value (i.e. length of the bar). This hopefully satisfies all
needs.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>

CHANGELOG.md

@@ -1,3 +1,4 @@
+Statistics: Implement ordering of categorical bar charts
 Profile: Fix editing of time / duration (profile of edited dive was not shown)
 Divelist: Don't attempt to compute SAC for CCR dives
 ---

desktop-widgets/statswidget.cpp

@@ -84,6 +84,7 @@ StatsWidget::StatsWidget(QWidget *parent) : QWidget(parent)
 	connect(ui.var1Binner, QOverload<int>::of(&QComboBox::activated), this, &StatsWidget::var1BinnerChanged);
 	connect(ui.var2Binner, QOverload<int>::of(&QComboBox::activated), this, &StatsWidget::var2BinnerChanged);
 	connect(ui.var2Operation, QOverload<int>::of(&QComboBox::activated), this, &StatsWidget::var2OperationChanged);
+	connect(ui.var1Sort, QOverload<int>::of(&QComboBox::activated), this, &StatsWidget::var1SortChanged);
 	connect(ui.restrictButton, &QToolButton::clicked, this, &StatsWidget::restrict);
 	connect(ui.unrestrictButton, &QToolButton::clicked, this, &StatsWidget::unrestrict);
 
@@ -129,6 +130,8 @@ void StatsWidget::updateUi()
 	setBinList(ui.var1Binner, uiState.binners1);
 	setBinList(ui.var2Binner, uiState.binners2);
 	setVariableList(ui.var2Operation, uiState.operations2);
+	setVariableList(ui.var1Sort, uiState.sortMode1);
+	ui.sortGroup->setVisible(!uiState.sortMode1.variables.empty());
 
 	// Add checkboxes for additional features
 	features.clear();
@@ -198,6 +201,12 @@ void StatsWidget::var2OperationChanged(int idx)
 	updateUi();
 }
 
+void StatsWidget::var1SortChanged(int idx)
+{
+	state.sortMode1Changed(ui.var1Sort->itemData(idx).toInt());
+	updateUi();
+}
+
 void StatsWidget::featureChanged(int idx, bool status)
 {
 	state.featureChanged(idx, status);

desktop-widgets/statswidget.h

@@ -24,6 +24,7 @@ slots:
 	void var1BinnerChanged(int);
 	void var2BinnerChanged(int);
 	void var2OperationChanged(int);
+	void var1SortChanged(int);
 	void featureChanged(int, bool);
 	void restrict();
 	void unrestrict();

desktop-widgets/statswidget.ui

@@ -95,6 +95,18 @@
        </layout>
       </widget>
      </item>
+     <item>
+      <widget class="QGroupBox" name="sortGroup">
+       <property name="title">
+        <string>Sorting</string>
+       </property>
+       <layout class="QVBoxLayout" name="chartLayout">
+        <item>
+         <widget class="QComboBox" name="var1Sort" />
+        </item>
+       </layout>
+      </widget>
+     </item>
      <item>
       <widget class="QGroupBox" name="restrictionGroup">
        <property name="title">

stats/pieseries.cpp

@@ -82,7 +82,7 @@ void PieSeries::Item::highlight(ChartPieItem &item, int bin_nr, bool highlight,
 }
 
 PieSeries::PieSeries(StatsView &view, StatsAxis *xAxis, StatsAxis *yAxis, const QString &categoryName,
-		     std::vector<std::pair<QString, std::vector<dive *>>> data, bool keepOrder) :
+		     std::vector<std::pair<QString, std::vector<dive *>>> data, ChartSortMode sortMode) :
 	StatsSeries(view, xAxis, yAxis),
 	item(view.createChartItem<ChartPieItem>(ChartZValue::Series, pieBorderWidth)),
 	categoryName(categoryName),
@@ -134,7 +134,7 @@ PieSeries::PieSeries(StatsView &view, StatsAxis *xAxis, StatsAxis *yAxis, const
 		++it;
 
 	// Sort the main groups and the other groups back, if requested
-	if (keepOrder) {
+	if (sortMode == ChartSortMode::Bin) {
 		std::sort(sorted.begin(), it);
 		std::sort(it, sorted.end());
 	}

stats/pieseries.h

@@ -15,6 +15,7 @@ struct InformationBox;
 class ChartPieItem;
 class ChartTextItem;
 class QRectF;
+enum class ChartSortMode : int;
 
 class PieSeries : public StatsSeries {
 public:
@@ -22,7 +23,7 @@ public:
 	// If keepOrder is false, bins will be sorted by size, otherwise the sorting
 	// of the shown bins will be retained. Small bins are omitted for clarity.
 	PieSeries(StatsView &view, StatsAxis *xAxis, StatsAxis *yAxis, const QString &categoryName,
-		  std::vector<std::pair<QString, std::vector<dive *>>> data, bool keepOrder);
+		  std::vector<std::pair<QString, std::vector<dive *>>> data, ChartSortMode sortMode);
 	~PieSeries();
 
 	void updatePositions() override;

stats/statsstate.cpp

@@ -3,7 +3,7 @@
 #include "statstranslations.h"
 #include "statsvariables.h"
 
-// Attn: The order must correspond to the enum above
+// Attn: The order must correspond to the enum ChartSubType
 static const char *chart_subtype_names[] = {
 	QT_TRANSLATE_NOOP("StatsTranslations", "vertical"),
 	QT_TRANSLATE_NOOP("StatsTranslations", "grouped vertical"),
@@ -16,6 +16,13 @@ static const char *chart_subtype_names[] = {
 	QT_TRANSLATE_NOOP("StatsTranslations", "piechart"),
 };
 
+// Attn: The order must correspond to the enum ChartSortMode
+static const char *sortmode_names[] = {
+	QT_TRANSLATE_NOOP("StatsTranslations", "Bin"),
+	QT_TRANSLATE_NOOP("StatsTranslations", "Count"),
+	QT_TRANSLATE_NOOP("StatsTranslations", "Value"),
+};
+
 enum class SupportedVariable {
 	None,
 	Categorical,		// Implies that the variable is binned
@@ -167,7 +174,8 @@ StatsState::StatsState() :
 	confidence(true),
 	var1Binner(nullptr),
 	var2Binner(nullptr),
-	var2Operation(StatsOperation::Invalid)
+	var2Operation(StatsOperation::Invalid),
+	sortMode1(ChartSortMode::Bin)
 {
 	validate(true);
 }
@@ -377,6 +385,40 @@ static std::vector<StatsState::Feature> createFeaturesList(int chartFeatures, co
 	return res;
 }
 
+// For creating the sort mode list, the ChartSortMode enum is misused to
+// indicate which sort modes are allowed:
+// 	bin -> none (list is redundant: only one mode)
+//	count -> bin, count
+//	value -> bin, count, value
+// In principle, the "highest possible" mode is given. If a mode is possible,
+// all the lower modes are likewise possible.
+static StatsState::VariableList createSortModeList(ChartSortMode allowed, ChartSortMode selectedSortMode)
+{
+	StatsState::VariableList res;
+	res.selected = -1;
+	if ((int)allowed <= (int)ChartSortMode::Bin)
+		return res;
+	for (int i = 0; i <= (int)allowed; ++i) {
+		ChartSortMode mode = static_cast<ChartSortMode>(i);
+		QString name = StatsTranslations::tr(sortmode_names[i]);
+		if (selectedSortMode == mode)
+			res.selected = (int)res.variables.size();
+		res.variables.push_back({ name, i });
+	}
+
+	return res;
+}
+
+static StatsState::VariableList createSortModeList1(ChartType type, ChartSortMode selectedSortMode)
+{
+	ChartSortMode allowed = ChartSortMode::Bin; // Default: no extra sorting
+	if (type == ChartType::DiscreteBar || type == ChartType::DiscreteCount || type == ChartType::Pie)
+		allowed = ChartSortMode::Count;
+	else if (type == ChartType::DiscreteValue)
+		allowed = ChartSortMode::Value;
+	return createSortModeList(allowed, selectedSortMode);
+}
+
 StatsState::UIState StatsState::getUIState() const
 {
 	UIState res;
@@ -390,6 +432,7 @@ StatsState::UIState StatsState::getUIState() const
 	res.binners2 = createBinnerList(var2, var2Binner, var1Binner != nullptr, true);
 	res.operations2 = createOperationsList(var2, var2Operation, var1Binner);
 	res.features = createFeaturesList(chartFeatures, *this);
+	res.sortMode1 = createSortModeList1(type, sortMode1);
 	return res;
 }
 
@@ -461,6 +504,12 @@ void StatsState::var2OperationChanged(int id)
 	validate(false);
 }
 
+void StatsState::sortMode1Changed(int id)
+{
+	sortMode1 = (ChartSortMode)id;
+	validate(false);
+}
+
 void StatsState::chartChanged(int id)
 {
 	std::tie(type, subtype) = fromInt(id); // use std::tie to assign two values at once
@@ -604,4 +653,12 @@ void StatsState::validate(bool varChanged)
 	// Median and mean currently only if the first variable is numeric
 	if (!var1 || var1->type() != StatsVariable::Type::Numeric)
 		chartFeatures &= ~(ChartFeatureMedian | ChartFeatureMean);
+
+	// By default, sort according to the used bin. Only for pie charts,
+	// sort by count, if the binning is on a categorical variable.
+	if (varChanged) {
+		sortMode1 = type == ChartType::Pie &&
+			    var1->type() == StatsVariable::Type::Discrete ? ChartSortMode::Count :
+									    ChartSortMode::Bin;
+	}
 }

stats/statsstate.h

@@ -36,6 +36,12 @@ enum class ChartSubType {
 	Count
 };
 
+enum class ChartSortMode {
+	Bin = 0,	// Sort according to the binner (i.e. don't change sorting)
+	Count,		// Sort by number of dives in the bin
+	Value		// Sort by the displayed value of the bin
+};
+
 struct ChartTypeDesc; // Internal implementation detail
 struct StatsVariable;
 struct StatsBinner;
@@ -84,6 +90,9 @@ public:
 		std::vector<Feature> features;
 		BinnerList binners1;
 		BinnerList binners2;
+		// Currently, only alternative sorting on the first variable.
+		// Sort mode reuses the variable list - not very nice.
+		VariableList sortMode1;
 		// Currently, operations are only supported on the second variable
 		// This reuses the variable list - not very nice.
 		VariableList operations2;
@@ -97,6 +106,7 @@ public:
 	void binner1Changed(int id);
 	void binner2Changed(int id);
 	void var2OperationChanged(int id);
+	void sortMode1Changed(int id);
 	void featureChanged(int id, bool state);
 
 	const StatsVariable *var1;	// Independent variable
@@ -113,6 +123,7 @@ public:
 	const StatsBinner *var1Binner;	// nullptr: undefined
 	const StatsBinner *var2Binner;	// nullptr: undefined
 	StatsOperation var2Operation;
+	ChartSortMode sortMode1;
 private:
 	void validate(bool varChanged);
 	int chartFeatures;

stats/statsview.cpp

@@ -553,15 +553,15 @@ void StatsView::plotChart()
 	}
 	switch (state.type) {
 	case ChartType::DiscreteBar:
-		return plotBarChart(dives, state.subtype, state.var1, state.var1Binner, state.var2,
+		return plotBarChart(dives, state.subtype, state.sortMode1, state.var1, state.var1Binner,
-				    state.var2Binner);
+				    state.var2, state.var2Binner);
 	case ChartType::DiscreteValue:
-		return plotValueChart(dives, state.subtype, state.var1, state.var1Binner, state.var2,
+		return plotValueChart(dives, state.subtype, state.sortMode1,
-				      state.var2Operation);
+				      state.var1, state.var1Binner, state.var2, state.var2Operation);
 	case ChartType::DiscreteCount:
-		return plotDiscreteCountChart(dives, state.subtype, state.var1, state.var1Binner);
+		return plotDiscreteCountChart(dives, state.subtype, state.sortMode1, state.var1, state.var1Binner);
 	case ChartType::Pie:
-		return plotPieChart(dives, state.var1, state.var1Binner);
+		return plotPieChart(dives, state.sortMode1, state.var1, state.var1Binner);
 	case ChartType::DiscreteBox:
 		return plotDiscreteBoxChart(dives, state.var1, state.var1Binner, state.var2);
 	case ChartType::DiscreteScatter:
@@ -709,7 +709,7 @@ static std::vector<BarSeries::MultiItem::Item> makeMultiItems(std::vector<std::v
 }
 
 void StatsView::plotBarChart(const std::vector<dive *> &dives,
-			     ChartSubType subType,
+			     ChartSubType subType, ChartSortMode sortMode,
 			     const StatsVariable *categoryVariable, const StatsBinner *categoryBinner,
 			     const StatsVariable *valueVariable, const StatsBinner *valueBinner)
 {
@@ -720,6 +720,13 @@ void StatsView::plotBarChart(const std::vector<dive *> &dives,
 
 	std::vector<StatsBinDives> categoryBins = categoryBinner->bin_dives(dives, false);
 
+	if (sortMode == ChartSortMode::Count) {
+		// Note: we sort by count in reverse order, as this is probably what the user desires(?).
+		std::sort(categoryBins.begin(), categoryBins.end(),
+			  [](const StatsBinDives &b1, const StatsBinDives &b2)
+			  { return b1.value.size() > b2.value.size(); });
+	}
+
 	bool isStacked = subType == ChartSubType::VerticalStacked || subType == ChartSubType::HorizontalStacked;
 	bool isHorizontal = subType == ChartSubType::HorizontalGrouped || subType == ChartSubType::HorizontalStacked;
 
@@ -820,7 +827,7 @@ static std::pair<double, double> getMinMaxValue(const std::vector<StatsBinOp> &b
 }
 
 void StatsView::plotValueChart(const std::vector<dive *> &dives,
-			       ChartSubType subType,
+			       ChartSubType subType, ChartSortMode sortMode,
 			       const StatsVariable *categoryVariable, const StatsBinner *categoryBinner,
 			       const StatsVariable *valueVariable, StatsOperation valueAxisOperation)
 {
@@ -835,6 +842,16 @@ void StatsView::plotValueChart(const std::vector<dive *> &dives,
 	if (categoryBins.empty())
 		return;
 
+	if (sortMode == ChartSortMode::Count) {
+		// Note: we sort by count in reverse order, as this is probably what the user desires(?).
+		std::sort(categoryBins.begin(), categoryBins.end(),
+			  [](const StatsBinOp &b1, const StatsBinOp &b2)
+			  { return b1.value.dives.size() > b2.value.dives.size(); });
+	} else if (sortMode == ChartSortMode::Value) {
+		std::sort(categoryBins.begin(), categoryBins.end(),
+			  [valueAxisOperation](const StatsBinOp &b1, const StatsBinOp &b2)
+			  { return b1.value.get(valueAxisOperation) < b2.value.get(valueAxisOperation); });
+	}
 
 	bool isHorizontal = subType == ChartSubType::Horizontal;
 	const auto [minValue, maxValue] = getMinMaxValue(categoryBins, valueAxisOperation);
@@ -885,7 +902,7 @@ static int getMaxCount(const std::vector<T> &bins)
 }
 
 void StatsView::plotDiscreteCountChart(const std::vector<dive *> &dives,
-				      ChartSubType subType,
+				      ChartSubType subType, ChartSortMode sortMode,
 				      const StatsVariable *categoryVariable, const StatsBinner *categoryBinner)
 {
 	if (!categoryBinner)
@@ -899,6 +916,13 @@ void StatsView::plotDiscreteCountChart(const std::vector<dive *> &dives,
 	if (categoryBins.empty())
 		return;
 
+	if (sortMode == ChartSortMode::Count) {
+		// Note: we sort by count in reverse order, as this is probably what the user desires(?).
+		std::sort(categoryBins.begin(), categoryBins.end(),
+			  [](const StatsBinDives &b1, const StatsBinDives &b2)
+			  { return b1.value.size() > b2.value.size(); });
+	}
+
 	int total = getTotalCount(categoryBins);
 	bool isHorizontal = subType != ChartSubType::Vertical;
 
@@ -926,7 +950,7 @@ void StatsView::plotDiscreteCountChart(const std::vector<dive *> &dives,
 	createSeries<BarSeries>(isHorizontal, categoryVariable->name(), std::move(items));
 }
 
-void StatsView::plotPieChart(const std::vector<dive *> &dives,
+void StatsView::plotPieChart(const std::vector<dive *> &dives, ChartSortMode sortMode,
 			     const StatsVariable *categoryVariable, const StatsBinner *categoryBinner)
 {
 	if (!categoryBinner)
@@ -945,8 +969,7 @@ void StatsView::plotPieChart(const std::vector<dive *> &dives,
 	for (auto &[bin, dives]: categoryBins)
 		data.emplace_back(categoryBinner->formatWithUnit(*bin), std::move(dives));
 
-	bool keepOrder = categoryVariable->type() != StatsVariable::Type::Discrete;
+	PieSeries *series = createSeries<PieSeries>(categoryVariable->name(), std::move(data), sortMode);
-	PieSeries *series = createSeries<PieSeries>(categoryVariable->name(), std::move(data), keepOrder);
 
 	legend = createChartItem<Legend>(series->binNames());
 }

stats/statsview.h

@@ -36,6 +36,7 @@ class RootNode;	// Internal implementation detail
 enum class ChartSubType : int;
 enum class ChartZValue : int;
 enum class StatsOperation : int;
+enum class ChartSortMode : int;
 
 class StatsView : public QQuickItem {
 	Q_OBJECT
@@ -81,17 +82,17 @@ private:
 	void reset(); // clears all series and axes
 	void setAxes(StatsAxis *x, StatsAxis *y);
 	void plotBarChart(const std::vector<dive *> &dives,
-			  ChartSubType subType,
+			  ChartSubType subType, ChartSortMode sortMode,
 			  const StatsVariable *categoryVariable, const StatsBinner *categoryBinner,
 			  const StatsVariable *valueVariable, const StatsBinner *valueBinner);
 	void plotValueChart(const std::vector<dive *> &dives,
-			    ChartSubType subType,
+			    ChartSubType subType, ChartSortMode sortMode,
 			    const StatsVariable *categoryVariable, const StatsBinner *categoryBinner,
 			    const StatsVariable *valueVariable, StatsOperation valueAxisOperation);
 	void plotDiscreteCountChart(const std::vector<dive *> &dives,
-				    ChartSubType subType,
+				    ChartSubType subType, ChartSortMode sortMode,
 				    const StatsVariable *categoryVariable, const StatsBinner *categoryBinner);
-	void plotPieChart(const std::vector<dive *> &dives,
+	void plotPieChart(const std::vector<dive *> &dives, ChartSortMode sortMode,
 			  const StatsVariable *categoryVariable, const StatsBinner *categoryBinner);
 	void plotDiscreteBoxChart(const std::vector<dive *> &dives,
 				  const StatsVariable *categoryVariable, const StatsBinner *categoryBinner, const StatsVariable *valueVariable);