From 6677fbdfbf2117db0ee5862298742ad8d11ea49b Mon Sep 17 00:00:00 2001 From: boutinb Date: Wed, 13 Nov 2024 17:25:52 +0100 Subject: [PATCH] Fix Variables List with layers for contingency table analysis Fixes https://github.com/jasp-stats/INTERNAL-jasp/issues/2683 I'm not sure if a Layer Variables List with several layers has ever worked properly. It was at least not possible to load a JASP file with a contingency table analysis with several layers. To reproduce the problem, you can also just duplicate the analysis. --- QMLComponents/models/listmodel.cpp | 35 +++- QMLComponents/models/listmodel.h | 3 + QMLComponents/models/listmodeldraggable.cpp | 26 --- QMLComponents/models/listmodeldraggable.h | 2 - .../models/listmodellayersassigned.cpp | 190 ++++++++++-------- .../models/listmodellayersassigned.h | 14 +- 6 files changed, 153 insertions(+), 117 deletions(-) diff --git a/QMLComponents/models/listmodel.cpp b/QMLComponents/models/listmodel.cpp index 153dc0ed85..9a76aa100a 100644 --- a/QMLComponents/models/listmodel.cpp +++ b/QMLComponents/models/listmodel.cpp @@ -454,15 +454,42 @@ int ListModel::rowCount(const QModelIndex &) const return int(terms().size()); } +Terms ListModel::termsFromIndexes(const QList &indexes) const +{ + Terms result; + + for (int index : indexes) + if (size_t(index) < terms().size()) + result.add(terms().at(index)); + + return result; +} + +QList ListModel::indexesFromTerms(const Terms & termsIn) const +{ + std::set result; + std::map termToIndex; + + for(size_t t=0; t= myTerms.size()) + Terms terms = termsFromIndexes({row}); + if (terms.size() == 0) return QVariant(); - const Term& term = myTerms.at(row_t); + const Term& term = terms[0]; switch (role) { diff --git a/QMLComponents/models/listmodel.h b/QMLComponents/models/listmodel.h index b5ba56ca99..d584982cf1 100644 --- a/QMLComponents/models/listmodel.h +++ b/QMLComponents/models/listmodel.h @@ -98,6 +98,9 @@ class ListModel : public QAbstractTableModel, public VariableInfoConsumer Terms checkTermsTypes(const Terms& terms) const; Terms checkTermsTypes(const std::vector& terms) const; + virtual Terms termsFromIndexes( const QList& indexes) const; + virtual QList indexesFromTerms( const Terms & terms) const; + Q_INVOKABLE int searchTermWith(QString searchString); Q_INVOKABLE void selectItem(int _index, bool _select); diff --git a/QMLComponents/models/listmodeldraggable.cpp b/QMLComponents/models/listmodeldraggable.cpp index ffd6c51d06..a21961ef5b 100644 --- a/QMLComponents/models/listmodeldraggable.cpp +++ b/QMLComponents/models/listmodeldraggable.cpp @@ -36,32 +36,6 @@ bool ListModelDraggable::keepTerms() const return variablesList ? variablesList->keepVariablesWhenMoved() : false; } -Terms ListModelDraggable::termsFromIndexes(const QList &indexes) const -{ - Terms result; - - for (int index : indexes) - if (size_t(index) < terms().size()) - result.add(terms().at(index)); - - return result; -} - -QList ListModelDraggable::indexesFromTerms(const Terms & termsIn) const -{ - std::set result; - std::map termToIndex; - - for(size_t t=0; t &indices) { if(!indices.count()) diff --git a/QMLComponents/models/listmodeldraggable.h b/QMLComponents/models/listmodeldraggable.h index 0b963a0494..63a6686195 100644 --- a/QMLComponents/models/listmodeldraggable.h +++ b/QMLComponents/models/listmodeldraggable.h @@ -37,12 +37,10 @@ class ListModelDraggable : public ListModel void setDropMode(JASPControl::DropMode dropMode) { _dropMode = dropMode; } - virtual QList indexesFromTerms( const Terms & terms) const; virtual Terms canAddTerms( const Terms & terms) const; virtual Terms addTerms( const Terms & terms, int dropItemIndex = -1, const RowControlsValues& rowValues = RowControlsValues()); virtual void moveTerms( const QList& indexes, int dropItemIndex = -1); virtual void removeTerms( const QList& indexes); - virtual Terms termsFromIndexes( const QList& indexes) const; protected: JASPControl::DropMode _dropMode = JASPControl::DropMode::DropNone; diff --git a/QMLComponents/models/listmodellayersassigned.cpp b/QMLComponents/models/listmodellayersassigned.cpp index 1f93d870d0..7ef8021432 100644 --- a/QMLComponents/models/listmodellayersassigned.cpp +++ b/QMLComponents/models/listmodellayersassigned.cpp @@ -17,8 +17,7 @@ // #include "listmodellayersassigned.h" -#include "boundcontrols/boundcontrollayers.h" -#include "utilities/qutils.h" +#include "controls/jasplistcontrol.h" using namespace std; @@ -32,13 +31,13 @@ void ListModelLayersAssigned::initLayers(const std::vector& variables : allVariables) { QList layer; for (const std::string& variable : variables) layer.push_back(QString::fromStdString(variable)); - _variables.push_back(layer); + _variablesPerLayer.push_back(layer); } _setTerms(); @@ -54,8 +53,9 @@ std::vector > > ListModelLayersAssigned::g std::vector > > layers; int layerNr = 0; - for (const QList& variables : _variables) + for (const QList& variables : _variablesPerLayer) { + layerNr++; std::vector layer; for (const QString& variable : variables) layer.push_back(variable.toStdString()); @@ -65,42 +65,32 @@ std::vector > > ListModelLayersAssigned::g return layers; } -int ListModelLayersAssigned::_getLayer(int index, int& indexInLayer, bool inclusive) const +std::pair ListModelLayersAssigned::_getLayer(int row, bool insertVariable) const { - int layer = 0; - int layerIndex = 0; // Layer 1 - indexInLayer = -1; + int rowCounter = 0, // count the rows until it gets the right layer + layer = 0, + variableIndiceInLayer = -1; - if (inclusive) index--; - while ((layer < _variables.length()) && (layerIndex + _variables[layer].length() < index)) + while ((layer < _variablesPerLayer.length()) && (rowCounter + _variablesPerLayer[layer].length() + (insertVariable ? 1 : 0) < row)) { - layerIndex += _variables[layer].length() + 1; + rowCounter += _variablesPerLayer[layer].length() + 1; // number of variables + layer header layer++; } - - if (inclusive) index++; - if (layer < _variables.length()) - indexInLayer = index - layerIndex - 1; + + if (layer < _variablesPerLayer.length()) + variableIndiceInLayer = row - rowCounter - 1; - return layer; + return make_pair(layer, variableIndiceInLayer); } void ListModelLayersAssigned::_setTerms() { + // Add only the variables in terms: as this is an assigned variables list, the terms must be in the available variables list + // But the available variables list does not have the layer headers. Terms newTerms; - int layer = 1; - for (const QList& variables : _variables) - { - newTerms.add(tr("Layer %1").arg(layer)); + for (const QList& variables : _variablesPerLayer) for (const QString& variable : variables) - { - Term term(variable, columnType::nominal); - newTerms.add(term); - } - layer++; - } - - newTerms.add(tr("Layer %1").arg(layer)); + newTerms.add(Term(variable, listView()->defaultType())); ListModel::_setTerms(newTerms); } @@ -114,15 +104,15 @@ Terms ListModelLayersAssigned::addTerms(const Terms& terms, int dropItemIndex, c beginResetModel(); - int layer = _variables.length(); + int layer = _variablesPerLayer.length(); int indexInLayer = 0; if (dropItemIndex >= 0) - layer = _getLayer(dropItemIndex, indexInLayer, true); + std::tie(layer, indexInLayer) = _getLayer(dropItemIndex, true); - if (layer >= _variables.length()) + if (layer >= _variablesPerLayer.length()) { - _variables.push_back(QList()); - layer = _variables.length() - 1; + _variablesPerLayer.push_back(QList()); + layer = _variablesPerLayer.length() - 1; indexInLayer = 0; } @@ -130,7 +120,7 @@ Terms ListModelLayersAssigned::addTerms(const Terms& terms, int dropItemIndex, c indexInLayer = 0; for (const Term& term : terms) - _variables[layer].insert(indexInLayer, term.asQString()); + _variablesPerLayer[layer].insert(indexInLayer, term.asQString()); _setTerms(); @@ -143,15 +133,15 @@ void ListModelLayersAssigned::moveTerms(const QList &indexes, int dropItemI { beginResetModel(); - int layerDrop = _variables.length(); + int layerDrop = _variablesPerLayer.length(); int indexInLayerDrop = 0; if (dropItemIndex >= 0) - layerDrop = _getLayer(dropItemIndex, indexInLayerDrop, true); + std::tie(layerDrop, indexInLayerDrop) = _getLayer(dropItemIndex, true); - if (layerDrop >= _variables.length()) + if (layerDrop >= _variablesPerLayer.length()) { - _variables.push_back(QList()); - layerDrop = _variables.length() - 1; + _variablesPerLayer.push_back(QList()); + layerDrop = _variablesPerLayer.length() - 1; indexInLayerDrop = 0; } @@ -161,42 +151,40 @@ void ListModelLayersAssigned::moveTerms(const QList &indexes, int dropItemI QList movedVariables; QList sortedIndexes = indexes; std::sort(sortedIndexes.begin(), sortedIndexes.end(), std::greater()); - // Store first the variables that must be moved, before removing them in the _variables list: + // Store first the variables that must be moved, before removing them in the variables list: // removing the items in the _variables list will change the indexes for (int index : sortedIndexes) { - int indexInLayer = 0; - int layer = _getLayer(index, indexInLayer); - - if (layer < _variables.length() && indexInLayer >= 0 && indexInLayer < _variables[layer].length()) - movedVariables.push_back(_variables[layer][indexInLayer]); + auto [layer, indexInLayer] = _getLayer(index); + + if (layer < _variablesPerLayer.length() && indexInLayer >= 0 && indexInLayer < _variablesPerLayer[layer].length()) + movedVariables.push_back(_variablesPerLayer[layer][indexInLayer]); } for (int index : sortedIndexes) { - int indexInLayer = 0; - int layer = _getLayer(index, indexInLayer); + auto [layer, indexInLayer] = _getLayer(index); - if (layer < _variables.length() && indexInLayer >= 0 && indexInLayer < _variables[layer].length()) + if (layer < _variablesPerLayer.length() && indexInLayer >= 0 && indexInLayer < _variablesPerLayer[layer].length()) { if (layer == layerDrop && indexInLayer < indexInLayerDrop) indexInLayerDrop--; - _variables[layer].removeAt(indexInLayer); + _variablesPerLayer[layer].removeAt(indexInLayer); } } for (const QString& variable : movedVariables) - _variables[layerDrop].insert(indexInLayerDrop, variable); + _variablesPerLayer[layerDrop].insert(indexInLayerDrop, variable); - for (int i = _variables.length() - 1; i >= 0; i--) + for (int i = _variablesPerLayer.length() - 1; i >= 0; i--) { - if (_variables[i].length() == 0) - _variables.removeAt(i); + if (_variablesPerLayer[i].length() == 0) + _variablesPerLayer.removeAt(i); } _setTerms(); - endResetModel(); + endResetModel(); } void ListModelLayersAssigned::removeTerms(const QList &indexes) @@ -212,24 +200,30 @@ void ListModelLayersAssigned::removeTerms(const QList &indexes) for (int index : sortedIndexes) { - int layer = _variables.length(); - int indexInLayer = 0; + auto [layer, indexInLayer] = _getLayer(index); - layer = _getLayer(index, indexInLayer); - if (layer >= 0 && layer < _variables.length() && indexInLayer >= 0 && indexInLayer < _variables[layer].length()) - _variables[layer].removeAt(indexInLayer); + if (layer >= 0 && layer < _variablesPerLayer.length() && indexInLayer >= 0 && indexInLayer < _variablesPerLayer[layer].length()) + _variablesPerLayer[layer].removeAt(indexInLayer); } - for (int i = _variables.length() - 1; i >= 0; i--) + for (int i = _variablesPerLayer.length() - 1; i >= 0; i--) { - if (_variables[i].length() == 0) - _variables.removeAt(i); + if (_variablesPerLayer[i].length() == 0) + _variablesPerLayer.removeAt(i); } _setTerms(); - endResetModel(); + endResetModel(); +} + +int ListModelLayersAssigned::rowCount(const QModelIndex &parent) const +{ + // terms().size() : number of variables + // _variablesPerLayer.count(): number of layers + // + 1 extra virtual layer + return terms().size() + _variablesPerLayer.count() + 1; } QVariant ListModelLayersAssigned::data(const QModelIndex &index, int role) const @@ -237,38 +231,74 @@ QVariant ListModelLayersAssigned::data(const QModelIndex &index, int role) const if ( ! index.isValid()) return QVariant(); - QVariant result; - - int indexInLayer = -1; - int layer = _getLayer(index.row(), indexInLayer); + auto [layer, indexInLayer] = _getLayer(index.row()); if (role == ListModel::SelectableRole) - { - result = (indexInLayer >= 0); - } + return (indexInLayer >= 0); else if (role == ListModel::TypeRole) { if (indexInLayer < 0) { QString type = "layer"; - if (layer == _variables.length()) + if (layer == _variablesPerLayer.length()) type += ",virtual"; - result = type; + return type; } else - result = "variable"; + return "variable"; } - else if (role == ListModel::ColumnTypeRole) + else if (indexInLayer == -1) { - if (layer >= 0 && layer < _variables.length() && indexInLayer >= 0 && indexInLayer < _variables[layer].length()) + if (role == Qt::DisplayRole || role == ListModel::NameRole) + return tr("Layer %1").arg(layer+1); + else if (role == ListModel::ColumnTypeRole) + return columnTypeToQString(columnType::unknown); + } + else + return ListModelAssignedInterface::data(index, role); + + return QVariant(); +} + +QList ListModelLayersAssigned::indexesFromTerms(const Terms &terms) const +{ + QList indexes; + + for (const Term& term : terms) + { + bool found = false; + int ind = 0; + for (auto & variables : _variablesPerLayer) { - QString variable = _variables[layer][indexInLayer]; - result = columnTypeToQString(getVariableType(variable)); + ind++; + for (const QString& var : variables) + { + if (var == term.asQString()) + { + found = true; + indexes.append(ind); + break; + } + ind++; + } + if (found) + break; + } } - else - result = ListModelAssignedInterface::data(index, role); + return indexes; +} + +Terms ListModelLayersAssigned::termsFromIndexes(const QList &indexes) const +{ + Terms result; + for (int i : indexes) + { + auto [layer, variableIndInLayer] = _getLayer(i); + if (variableIndInLayer >= 0) + result.add(_variablesPerLayer[layer][variableIndInLayer]); + } return result; } diff --git a/QMLComponents/models/listmodellayersassigned.h b/QMLComponents/models/listmodellayersassigned.h index 6b69f1e8c6..55367cd384 100644 --- a/QMLComponents/models/listmodellayersassigned.h +++ b/QMLComponents/models/listmodellayersassigned.h @@ -24,6 +24,7 @@ class ListModelLayersAssigned : public ListModelAssignedInterface { Q_OBJECT + public: ListModelLayersAssigned(JASPListControl* listView); @@ -31,15 +32,18 @@ class ListModelLayersAssigned : public ListModelAssignedInterface Terms addTerms(const Terms& terms, int dropItemIndex = -1, const RowControlsValues& rowValues = RowControlsValues()) override; void moveTerms(const QList& indexes, int dropItemIndex = -1) override; void removeTerms(const QList& indexes) override; - + int rowCount(const QModelIndex &parent = QModelIndex()) const override; + QList indexesFromTerms(const Terms &terms) const override; + Terms termsFromIndexes(const QList &indexes) const override; + void initLayers(const std::vector >& allVariables); std::vector > > getLayers() const; - + private: - int _getLayer(int index, int& realIndex, bool inclusive = false) const; + std::pair _getLayer(int index, bool insertVariable = false) const; void _setTerms(); - - QList > _variables; + + QList > _variablesPerLayer; }; #endif // LISTMODELLAYERSASSIGNED_H