fluid.plotter: correctly construct unique labels

[jamesb93]

This is something Rod found in the Max Beta and well it is certainly weird that it worked at all for so long... I think.

Old

function constructColorScheme() {
	if (labelDict) {
		const data = labelDict.get('data');
		const keys = data.getkeys();
		var uniques = new Array();

		// How many unique labels are there?
		keys.forEach(function(key) {
			var label = data.get(key);
			if (uniques.indexOf(label) == -1) {
				uniques.push(label)
			}
		})
		
		colorMap = {};
		var scheme = strChunk(_colorscheme, 6);
		uniques.sort();
		uniques.forEach(function(u, i) {
			i = i % scheme.length;
			var color = hexToRGB(scheme[i], 1.0);
			colorMap[u] = color;
		});

		// Strip any of the points from the pointColors if they have been assigned here
		keys.forEach(function(pt) {
			if (pointColors.hasOwnProperty(pt)) {
				delete pointColors[pt]
			}
		})
		
		mgraphics.redraw();
	}
}

New

function constructColorScheme() {
	if (labelDict) {
		var labelDictAsJson = JSON.parse(labelDict.stringify());
		var data = labelDictAsJson.data;
		var keys = Object.keys(data)
		var uniques = []

		// How many unique labels are there?
		keys.forEach(function(key) {
			var label = data[key]
			if (uniques.indexOf(label) == -1) {
				uniques.push(parseInt(label[0]))
			}
		})
		uniques = uniques.removeDuplicates()
		uniques.sort();

		var scheme = strChunk(_colorscheme, 6);
		colorMap = {}
		colorMap = uniques.reduce(function (map, u, i) {
			const color = hexToRGB(scheme[i % scheme.length], 1.0);
			map[u] = color;
			return map;
		}, {});

		// Strip any of the points from the pointColors if they have been assigned here
		keys.forEach(function(pt) {
			if (pointColors.hasOwnProperty(pt)) {
				delete pointColors[pt]
			}
		})
		
		mgraphics.redraw();
	}
}

Array.prototype.removeDuplicates = function() {
	var uniqueArray = [];
	for (var i = 0; i < this.length; i++) {
		if (uniqueArray.indexOf(this[i]) === -1) {
			uniqueArray.push(this[i]);
		}
	}
	return uniqueArray;
};

At first I suspected maybe I was treating dictionaries incorrectly (with .get() and friends) but that was quickly snuffed out when I changed everything to use JSON representations (and found it to produce exactly the same behaviour). I then stepped through the code manually with a small test case and was intrigued why the programmer (💀) initially wrote this bit of code...

		// How many unique labels are there?
		keys.forEach(function(key) {
			var label = data.get(key);
			if (uniques.indexOf(label) == -1) {
				uniques.push(label)
			}
		})

label is not a number or a string as I assumed it to be but is rather an object... Thus the sorting, sort of works accidentally in most cases but also there are duplicate uniques 🤦. When you get to constructing the colour map you are iterating over the colours and replacing them because the index is not necessarily any more tied to the instance of when the loop has first seen a colour. Durp.

[tremblap]

there is a line I don't understand why it is there?

[tremblap]

why do we need to run this here? the keys.forEach is not supposed to add anything that is found, right?

[jamesb93]

In theory you should not need it, but it makes for a good guard against the original problem emerging.

It is very cheap to run so I would opt to just keep it, even though it is redundant.

[jamesb93]

Actually yeah looking at that its very meaningless to put this into an array prototype function.

I did update the check to actually work, which is why I think I needed the removal of duplicates: a2f90e0

[tremblap]

indeed high priority. I've removed the code that is redundant since the first iteration does not allow for duplicates. Please test on your side and let me know if that works as intended.

[jamesb93]

I'll test again and then get back to you :)