Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Auto Sort labels bug and other label handling issues #5630

Closed
wants to merge 1 commit into from
Closed

Auto Sort labels bug and other label handling issues #5630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 77 additions & 32 deletions CommonData/column.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ void Column::setAutoSortByValuesByDefault(bool autoSort)
_autoSortByValuesByDefault = autoSort;
}

Column::Column(DataSet * data, int id)
Column::Column(DataSet * data, int id, columnType colType, computedColumnType computedType, bool autoSort)
: DataSetBaseNode(dataSetBaseNodeType::column, data->dataNode()),
_data( data),
_id( id),
Expand All @@ -29,19 +29,32 @@ Column::Column(DataSet * data, int id)
db().columnSetAutoSort(_id, _autoSortByValue); //Store autosort in db
}

Column::~Column()

Column* Column::addColumn(DataSet * data, int index, const std::string& name, columnType colType, computedColumnType computedType, bool alterDataSetTable)
{
labelsTempReset();
delete _emptyValues;
delete _doubleDummy;
int id = data->db().columnInsert(data->id(), index, colType, computedType, Column::autoSortByValuesByDefault());
Column* col = new Column(data, id, colType, computedType, Column::autoSortByValuesByDefault());

if (!name.empty())
col->setName(name);

return col;
}

void Column::dbCreate(int index)
Column* Column::loadColumn(DataSet * data, int index)
{
JASPTIMER_SCOPE(Column::dbCreate);
Column* col = new Column(data, index, columnType::unknown, computedColumnType::notComputed, Column::autoSortByValuesByDefault());
col->dbLoadIndex(index, false);

assert(_id == -1);
db().columnInsert(_id, index);
return col;
}


Column::~Column()
{
labelsTempReset();
delete _emptyValues;
delete _doubleDummy;
}

void Column::dbLoad(int id, bool getValues)
Expand Down Expand Up @@ -391,18 +404,17 @@ columnTypeChangeResult Column::changeType(columnType colType)
if(codeType() == computedColumnType::analysis)
return columnTypeChangeResult::generatedFromAnalysis;

setDefaultValues(colType);
if (colType != columnType::unknown)
setType(colType);
setDefaultValues();
invalidate();
return columnTypeChangeResult::changed;
}
}

void Column::setDefaultValues(enum columnType columnType)
void Column::setDefaultValues()
{
JASPTIMER_SCOPE(Column::setDefaultValues);

if(columnType != columnType::unknown)
setType(columnType);

for(size_t i=0; i<_ints.size(); i++)
{
Expand Down Expand Up @@ -1719,7 +1731,7 @@ void Column::labelsOrderByValue(bool doDbUpdateEtc)
bool replaceAllDoubles = false;
static double dummy;

for(Label * label : labels())
for(Label * label : labels())
if(!label->isEmptyValue() && !(label->originalValue().isDouble() || ColumnUtils::getDoubleValue(label->originalValueAsString(), dummy)))
{
replaceAllDoubles = true;
Expand All @@ -1729,24 +1741,43 @@ void Column::labelsOrderByValue(bool doDbUpdateEtc)
if(replaceAllDoubles)
replaceDoublesTillLabelsRowWithLabels(labelsTempCount());

doublevec asc = valuesNumericOrdered();
size_t curMax = asc.size()+1;
std::map<double, int> orderMap;

for(size_t i=0; i<asc.size(); i++)
orderMap[asc[i]] = i;

//and now to write them back into the data
for(Label * label : _labels)
doublevec asc = valuesNumericOrdered();

if (asc.empty())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So with this change it either orders the numeric ones or the non-numerics if there are no numerics?

Why not order both?

{
double aValue = EmptyValues::missingValueDouble;

if(label->originalValue().isDouble())
aValue = label->originalValue().asDouble();
else
ColumnUtils::getDoubleValue(label->originalValueAsString(), aValue);

label->setOrder(!std::isnan(aValue) ? orderMap[aValue] : curMax++);
stringvec orderedstrings = valuesAlphabeticOrdered();
size_t curMax = orderedstrings.size()+1;
std::map<std::string, int> orderMap;

for(size_t i=0; i<orderedstrings.size(); i++)
orderMap[orderedstrings[i]] = i;

for(Label * label : _labels)
{
std::string aValue = label->originalValueAsString();
label->setOrder(!isEmptyValue(aValue) ? orderMap[aValue] : curMax++);
}
}
else
{
size_t curMax = asc.size()+1;
std::map<double, int> orderMap;

for(size_t i=0; i<asc.size(); i++)
orderMap[asc[i]] = i;

//and now to write them back into the data
for(Label * label : _labels)
{
double aValue = EmptyValues::missingValueDouble;

if(label->originalValue().isDouble())
aValue = label->originalValue().asDouble();
else
ColumnUtils::getDoubleValue(label->originalValueAsString(), aValue);

label->setOrder(!std::isnan(aValue) ? orderMap[aValue] : curMax++);
}
}

_sortLabelsByOrder();
Expand Down Expand Up @@ -1775,6 +1806,20 @@ doublevec Column::valuesNumericOrdered()
return doublevec(values.begin(), values.end());
}

stringvec Column::valuesAlphabeticOrdered()
{
stringset values;

for(const Label * label : _labels)
{
std::string aValue = label->originalValueAsString();
if (!isEmptyValue(aValue))
values.insert(aValue);
}

return stringvec(values.begin(), values.end());
}

void Column::valuesReverse()
{
JASPTIMER_SCOPE(Column::valuesReverse);
Expand Down
12 changes: 9 additions & 3 deletions CommonData/column.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,17 @@ class Column : public DataSetBaseNode
typedef std::map<std::pair<std::string, std::string>, Label*> LabelByStrStr;

Column(DataSet * data, int id = -1);
private:
Column(DataSet * data, int id, columnType colType, computedColumnType computedType, bool autoSort);

public:
static Column * addColumn(DataSet* data, int index = -1, const std::string & name = "", columnType colType = columnType::scale, computedColumnType computedType = computedColumnType::notComputed, bool alterDataSetTable = true);
static Column * loadColumn(DataSet* data, int index);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function obviously should be on DataSet...

~Column();

DatabaseInterface & db();
const DatabaseInterface & db() const;

void dbCreate( int index);
void dbLoad( int id=-1, bool getValues = true); ///< Loads *and* reloads from DB!
void dbLoadIndex(int index, bool getValues = true);
void dbUpdateComputedColumnStuff();
Expand All @@ -72,7 +77,7 @@ class Column : public DataSetBaseNode
void setIndex( int index );
void setInvalidated( bool invalidated );
void setCompColStuff( bool invalidated, bool forceSourceColType, computedColumnType codeType, const std::string & rCode, const std::string & error, const Json::Value & constructorJson);
void setDefaultValues( enum columnType columnType = columnType::unknown);
void setDefaultValues();

bool setAsNominalOrOrdinal( const intvec & values, bool is_ordinal = false);
bool setAsNominalOrOrdinal( const intvec & values, intstrmap uniqueValues, bool is_ordinal = false);
Expand Down Expand Up @@ -242,7 +247,8 @@ class Column : public DataSetBaseNode
columnTypeChangeResult _changeColumnToScale();
void _convertVectorIntToDouble(intvec & intValues, doublevec & doubleValues);
void _resetLabelValueMap();
doublevec valuesNumericOrdered();
doublevec valuesNumericOrdered();
stringvec valuesAlphabeticOrdered();

private:
DataSet * const _data;
Expand Down
15 changes: 9 additions & 6 deletions CommonData/databaseinterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,11 +441,11 @@ void DatabaseInterface::filterWrite(int filterIndex, const std::vector<bool> & v
transactionWriteEnd();
}

int DatabaseInterface::columnInsert(int dataSetId, int index, const std::string & name, columnType colType, bool alterTable)
int DatabaseInterface::columnInsert(int dataSetId, int index, columnType colType, computedColumnType computedType, bool autoSort, bool alterTable)
{
JASPTIMER_SCOPE(DatabaseInterface::columnInsert);
transactionWriteBegin();

if(index == -1) index = columnLastFreeIndex(dataSetId);
else columnIndexIncrements(dataSetId, index);

Expand All @@ -454,14 +454,17 @@ int DatabaseInterface::columnInsert(int dataSetId, int index, const std::string
#endif

//Create column entry
int columnId = runStatementsId("INSERT INTO Columns (dataSet, name, columnType, colIdx, analysisId) VALUES (?, ?, ?, ?, -1) RETURNING id;", [&](sqlite3_stmt * stmt)
int columnId = runStatementsId("INSERT INTO Columns (dataSet, columnType, codeType, autoSortByValue, colIdx, analysisId) VALUES (?, ?, ?, ?, ?, -1) RETURNING id;", [&](sqlite3_stmt * stmt)
{
sqlite3_bind_int(stmt, 1, dataSetId);
sqlite3_bind_text(stmt, 2, name.c_str(), name.length(), SQLITE_TRANSIENT);

std::string colT = columnTypeToString(colType);
sqlite3_bind_text(stmt, 3, colT.c_str(), colT.length(), SQLITE_TRANSIENT);
sqlite3_bind_int(stmt, 4, index);
std::string codeT = computedColumnTypeToString(computedType);

sqlite3_bind_text(stmt, 2, colT.c_str(), colT.length(), SQLITE_TRANSIENT);
sqlite3_bind_text(stmt, 3, codeT.c_str(), codeT.length(), SQLITE_TRANSIENT);
sqlite3_bind_int(stmt, 4, autoSort);
sqlite3_bind_int(stmt, 5, index);
});

#ifdef SIR_LOG_A_LOT
Expand Down
2 changes: 1 addition & 1 deletion CommonData/databaseinterface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ class DatabaseInterface

//Columns & Data/Values
//Index stuff:
int columnInsert( int dataSetId, int index = -1, const std::string & name = "", columnType colType = columnType::unknown, bool alterTable=true); ///< Insert a row into Columns and create the corresponding columns in DataSet_? Also makes sure the indices are correct
int columnInsert( int dataSetId, int index, columnType colType, computedColumnType computedType, bool autoSort, bool alterTable=true); ///< Insert a row into Columns and create the corresponding columns in DataSet_? Also makes sure the indices are correct
int columnLastFreeIndex( int dataSetId);
void columnIndexIncrements( int dataSetId, int index); ///< If index already is in use that column and all after are incremented by 1
void columnIndexDecrements( int dataSetId, int index); ///< Indices bigger than index are decremented, assumption is that the previous one using it has been removed already
Expand Down
17 changes: 9 additions & 8 deletions CommonData/dataset.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,25 +172,26 @@ void DataSet::removeColumn(const std::string & name)
}
}

void DataSet::insertColumn(size_t index, bool alterDataSetTable)
Column* DataSet::insertColumn(size_t index, bool alterDataSetTable, const std::string & name, columnType colType, computedColumnType computedType)
{

assert(_dataSetID > 0);

Column * newColumn = new Column(this, db().columnInsert(_dataSetID, index, "", columnType::unknown, alterDataSetTable));
Column * newColumn = Column::addColumn(this, index, name, colType, computedType, alterDataSetTable);

_columns.insert(_columns.begin()+index, newColumn);

newColumn->setRowCount(_rowCount);
newColumn->setDefaultValues();

incRevision();

return newColumn;
}

Column * DataSet::newColumn(const std::string &name)
{
assert(_dataSetID > 0);
Column * col = new Column(this, db().columnInsert(_dataSetID, -1, name));
col->setName(name);
Column * col = Column::addColumn(this, -1, name);

_columns.push_back(col);

Expand Down Expand Up @@ -281,9 +282,9 @@ void DataSet::dbLoad(int index, std::function<void(float)> progressCallback, boo
for(size_t i=0; i<colCount; i++)
{
if(_columns.size() == i)
_columns.push_back(new Column(this));

_columns[i]->dbLoadIndex(i, false);
_columns.push_back(Column::loadColumn(this, i));
else
_columns[i]->dbLoadIndex(i, false);

progressCallback(0.2 + (i * colProgressMult * 0.3)); //should end at 0.5
}
Expand Down
2 changes: 1 addition & 1 deletion CommonData/dataset.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class DataSet : public DataSetBaseNode
void removeColumn( const std::string & name );
void removeColumn( size_t index );
void removeColumnById( size_t id );
void insertColumn( size_t index, bool alterDataSetTable = true);
Column * insertColumn(size_t index, bool alterDataSetTable = true, const std::string & name = "", columnType colType = columnType::unknown, computedColumnType computedType = computedColumnType::notComputed);
Column * newColumn( const std::string & name);
int getColumnIndex( const std::string & name ) const;
int columnIndex( const Column * col ) const;
Expand Down
32 changes: 16 additions & 16 deletions Desktop/components/JASP/Widgets/LabelEditorWindow.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,15 @@ FocusScope
target: columnModel
function onChosenColumnChanged()
{
levelsTableView.lastRow = -1;
levelsTableView.selectedRow = -1;
}
}

property real filterColWidth: 60 * jaspTheme.uiScale
property real remainingWidth: width - filterColWidth
property real valueColWidth: Math.min(columnModel.valueMaxWidth + 10, remainingWidth * 0.5) * jaspTheme.uiScale
property real labelColWidth: Math.min(columnModel.labelMaxWidth + 10, remainingWidth * 0.5) * jaspTheme.uiScale
property int lastRow: -1
property int selectedRow: -1


columnHeaderDelegate: Item
Expand Down Expand Up @@ -129,7 +129,7 @@ FocusScope
{
id: backgroundItem

onActiveFocusChanged: if(activeFocus) levelsTableView.lastRow = rowIndex
onActiveFocusChanged: if(activeFocus) levelsTableView.selectedRow = rowIndex

MouseArea
{
Expand Down Expand Up @@ -440,7 +440,7 @@ FocusScope
iconSource: jaspTheme.iconPath + "menu-column-order-by-values.svg"
onClicked: { forceActiveFocus(); columnModel.toggleAutoSortByValues(); }

toolTip: qsTr("Automatically order labels by their numeric value")
toolTip: qsTr("Automatically order labels by their value")

height: buttonColumnVariablesWindow.buttonHeight
implicitHeight: buttonColumnVariablesWindow.buttonHeight
Expand All @@ -458,49 +458,49 @@ FocusScope
height: buttonColumnVariablesWindow.buttonHeight
implicitHeight: buttonColumnVariablesWindow.buttonHeight
width: height
visible: !columnModel.autoSort || columnModel.firstNonNumericRow > 1 //if there are at least 2 numerics we have something to reverse
visible: columnModel.hasSeveralNumericValues //if there are at least 2 numerics we have something to reverse
}

RoundedButton
{
iconSource: jaspTheme.iconPath + "arrow-reverse.png"
onClicked: { forceActiveFocus(); columnModel.reverse(); }

toolTip: columnModel.autoSort ? qsTr("Reverse order of the labels with non-numeric values") : qsTr("Reverse order of all labels")
toolTip: qsTr("Reverse order of all labels")

height: buttonColumnVariablesWindow.buttonHeight
implicitHeight: buttonColumnVariablesWindow.buttonHeight
width: height
visible: !columnModel.autoSort || columnModel.rowsTotal - columnModel.firstNonNumericRow > 1 //If there are at least 2 non numerics there is something to reverse

visible: !columnModel.autoSort
enabled: columnModel.rowCount() > 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont use rowCount() in a binding because it wont update on changes...

}

RoundedButton
{
iconSource: jaspTheme.iconPath + "arrow-up.png"

onClicked: { forceActiveFocus(); columnModel.moveSelectionUp(); levelsTableView.lastRow--; }
toolTip: columnModel.autoSort ? qsTr("Move selected non-numeric labels up") : qsTr("Move selected labels up")
onClicked: { forceActiveFocus(); columnModel.moveSelectionUp(); levelsTableView.selectedRow--; }
toolTip: qsTr("Move selected labels up")

height: buttonColumnVariablesWindow.buttonHeight
implicitHeight: buttonColumnVariablesWindow.buttonHeight
width: height
enabled: levelsTableView.lastRow == -1 ? false : columnModel.firstNonNumericRow < levelsTableView.lastRow
visible: !columnModel.autoSort || columnModel.rowsTotal - columnModel.firstNonNumericRow > 1 //If there are at least 2 non numerics there is something to move up
enabled: levelsTableView.selectedRow > 0
visible: !columnModel.autoSort
}

RoundedButton
{
iconSource: jaspTheme.iconPath + "arrow-down.png"

onClicked: { forceActiveFocus(); columnModel.moveSelectionDown(); levelsTableView.lastRow++; }
toolTip: columnModel.autoSort ? qsTr("Move selected non-numeric labels down") : qsTr("Move selected labels down")
onClicked: { forceActiveFocus(); columnModel.moveSelectionDown(); levelsTableView.selectedRow++; }
toolTip: qsTr("Move selected labels down")

height: buttonColumnVariablesWindow.buttonHeight
implicitHeight: buttonColumnVariablesWindow.buttonHeight
width: height
enabled: levelsTableView.lastRow == -1 ? false : ((columnModel.firstNonNumericRow <= levelsTableView.lastRow) && ( levelsTableView.lastRow < columnModel.rowsTotal - 1 ))
visible: !columnModel.autoSort || columnModel.rowsTotal - columnModel.firstNonNumericRow > 1 //If there are at least 2 non numerics there is something to move down
enabled: levelsTableView.selectedRow >= 0 && levelsTableView.selectedRow < columnModel.rowCount() - 1
visible: !columnModel.autoSort
}

RoundedButton
Expand Down
Loading
Loading