From 1e5191e33ef7ec3f112be8091ea8602940d18ec3 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Fri, 31 Dec 2021 18:29:06 +0100 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + desktop-widgets/statswidget.cpp | 9 +++++ desktop-widgets/statswidget.h | 1 + desktop-widgets/statswidget.ui | 12 +++++++ stats/pieseries.cpp | 4 +-- stats/pieseries.h | 3 +- stats/statsstate.cpp | 61 +++++++++++++++++++++++++++++++-- stats/statsstate.h | 11 ++++++ stats/statsview.cpp | 47 ++++++++++++++++++------- stats/statsview.h | 9 ++--- 10 files changed, 137 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a0d33a7b..e9c737119 100644 --- a/CHANGELOG.md +++ b/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 --- diff --git a/desktop-widgets/statswidget.cpp b/desktop-widgets/statswidget.cpp index 6b936915f..9441fe3f2 100644 --- a/desktop-widgets/statswidget.cpp +++ b/desktop-widgets/statswidget.cpp @@ -84,6 +84,7 @@ StatsWidget::StatsWidget(QWidget *parent) : QWidget(parent) connect(ui.var1Binner, QOverload::of(&QComboBox::activated), this, &StatsWidget::var1BinnerChanged); connect(ui.var2Binner, QOverload::of(&QComboBox::activated), this, &StatsWidget::var2BinnerChanged); connect(ui.var2Operation, QOverload::of(&QComboBox::activated), this, &StatsWidget::var2OperationChanged); + connect(ui.var1Sort, QOverload::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); diff --git a/desktop-widgets/statswidget.h b/desktop-widgets/statswidget.h index c3397d613..93378d373 100644 --- a/desktop-widgets/statswidget.h +++ b/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(); diff --git a/desktop-widgets/statswidget.ui b/desktop-widgets/statswidget.ui index fa83cffcb..f322cd2d7 100644 --- a/desktop-widgets/statswidget.ui +++ b/desktop-widgets/statswidget.ui @@ -95,6 +95,18 @@ + + + + Sorting + + + + + + + + diff --git a/stats/pieseries.cpp b/stats/pieseries.cpp index b61f043e1..085b4d28c 100644 --- a/stats/pieseries.cpp +++ b/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>> data, bool keepOrder) : + std::vector>> data, ChartSortMode sortMode) : StatsSeries(view, xAxis, yAxis), item(view.createChartItem(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()); } diff --git a/stats/pieseries.h b/stats/pieseries.h index 5bd184dae..95adf32c0 100644 --- a/stats/pieseries.h +++ b/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>> data, bool keepOrder); + std::vector>> data, ChartSortMode sortMode); ~PieSeries(); void updatePositions() override; diff --git a/stats/statsstate.cpp b/stats/statsstate.cpp index 65d5c8656..c89c0b4ab 100644 --- a/stats/statsstate.cpp +++ b/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 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(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; + } } diff --git a/stats/statsstate.h b/stats/statsstate.h index 8fa6bb176..4fac2cd93 100644 --- a/stats/statsstate.h +++ b/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 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; diff --git a/stats/statsview.cpp b/stats/statsview.cpp index 148611cfc..84ed7d402 100644 --- a/stats/statsview.cpp +++ b/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, - state.var2Binner); + return plotBarChart(dives, state.subtype, state.sortMode1, state.var1, state.var1Binner, + state.var2, state.var2Binner); case ChartType::DiscreteValue: - return plotValueChart(dives, state.subtype, state.var1, state.var1Binner, state.var2, - state.var2Operation); + return plotValueChart(dives, state.subtype, state.sortMode1, + 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 makeMultiItems(std::vector &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 &dives, std::vector 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 getMinMaxValue(const std::vector &b } void StatsView::plotValueChart(const std::vector &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 &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 &bins) } void StatsView::plotDiscreteCountChart(const std::vector &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 &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 &dives, createSeries(isHorizontal, categoryVariable->name(), std::move(items)); } -void StatsView::plotPieChart(const std::vector &dives, +void StatsView::plotPieChart(const std::vector &dives, ChartSortMode sortMode, const StatsVariable *categoryVariable, const StatsBinner *categoryBinner) { if (!categoryBinner) @@ -945,8 +969,7 @@ void StatsView::plotPieChart(const std::vector &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(categoryVariable->name(), std::move(data), keepOrder); + PieSeries *series = createSeries(categoryVariable->name(), std::move(data), sortMode); legend = createChartItem(series->binNames()); } diff --git a/stats/statsview.h b/stats/statsview.h index 204d4f568..11a155156 100644 --- a/stats/statsview.h +++ b/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 &dives, - ChartSubType subType, + ChartSubType subType, ChartSortMode sortMode, const StatsVariable *categoryVariable, const StatsBinner *categoryBinner, const StatsVariable *valueVariable, const StatsBinner *valueBinner); void plotValueChart(const std::vector &dives, - ChartSubType subType, + ChartSubType subType, ChartSortMode sortMode, const StatsVariable *categoryVariable, const StatsBinner *categoryBinner, const StatsVariable *valueVariable, StatsOperation valueAxisOperation); void plotDiscreteCountChart(const std::vector &dives, - ChartSubType subType, + ChartSubType subType, ChartSortMode sortMode, const StatsVariable *categoryVariable, const StatsBinner *categoryBinner); - void plotPieChart(const std::vector &dives, + void plotPieChart(const std::vector &dives, ChartSortMode sortMode, const StatsVariable *categoryVariable, const StatsBinner *categoryBinner); void plotDiscreteBoxChart(const std::vector &dives, const StatsVariable *categoryVariable, const StatsBinner *categoryBinner, const StatsVariable *valueVariable);