Auto Sort labels does bug and other label handling issues

Author: boutinb

CommonData/column.cpp

@@ -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),
@@ -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)
@@ -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++)
 	{
@@ -1723,7 +1735,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;
@@ -1733,24 +1745,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())
 	{
-		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();
@@ -1779,6 +1810,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);

CommonData/column.h

@@ -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);
 									~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();
@@ -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);
@@ -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;

CommonData/databaseinterface.cpp

@@ -398,11 +398,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);
 
@@ -411,14 +411,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

CommonData/databaseinterface.h

@@ -110,7 +110,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

CommonData/dataset.cpp

@@ -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);
 
@@ -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
 	}

CommonData/dataset.h

@@ -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;

Desktop/components/JASP/Widgets/LabelEditorWindow.qml

@@ -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
@@ -129,7 +129,7 @@ FocusScope
 				{
 					id:						backgroundItem
 
-					onActiveFocusChanged:	if(activeFocus)	levelsTableView.lastRow = rowIndex
+					onActiveFocusChanged:	if(activeFocus)	levelsTableView.selectedRow = rowIndex
 
 					MouseArea
 					{
@@ -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
@@ -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
 				}
 
 				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