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.

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

Fix bounds matching for zero values #1643

Merged
merged 1 commit into from
Mar 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
47 changes: 29 additions & 18 deletions src/util/processFrequencies.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,44 @@ import { getTraitFromNode } from "../util/treeMiscHelpers";
export const unassigned_label = "unassigned";

/**
* assign a given node to a category
* Find the colorBy value of the node and, using the colorScale and the provided categories,
* assign the correct category.
* @return {string} category or the unassigned label
* Assign a given node to a category for the current colorScale / colorBy.
* Categories are legend values for the given coloring (or potentially
* a subset of legend values) and therefore will have a corresponding `colorScale.legendBounds`.
* A node value, `v`, is assigned to a category with legend bounds {a,b} if `v \in (a, b]`
* unless we are in the first category in which case it's `v \in [a, b]` (to consider the `v=a` case)
* See also `determineLegendMatch`.
* @return {any} element of `categories` OR `unassigned_label`
*/
const assignCategory = (colorScale, categories, node, colorBy, isGenotype) => {
if (isGenotype) return node.currentGt;
const value = getTraitFromNode(node, colorBy);
if (!value || value === "unknown") {
return unassigned_label;

if (!colorScale.continuous) {
/* Note - the following conditional previously applied to continuous scales
as well, but caused bugs in cases such as `value=0`. We should centralise and clarify
what we mean by unknown values as there may well be bugs in here -- for instance
can ordinal scales have `value=0`? I'm leaving it here for now to preserve previous
behavior for non-continous scales james, march 2023 */
if (!value || value === "unknown") {
return unassigned_label;
}
return value;
}
if (!colorScale.continuous) return value;

for (let i = 0; i < categories.length; i++) {
/* roughly the same logic as the determineLegendMatch function */
const category = categories[i];
if (category === unassigned_label) {
return unassigned_label;
}
const lowerBound = colorScale.legendBounds[category][0];
const upperBound = colorScale.legendBounds[category][1];
if (value <= upperBound && value > lowerBound) {
/* the following looks like it's checking `value \in [a,b]` however because category values
are ascending the case where `value=a` can only occur for the first category (i=0). There
is an assumption here that `b_i = a_{i+1}`; if this is not true then there is a bug in
the generation of the bounds. */
if (value <= upperBound && value >= lowerBound) {
return category;
}
}
console.error("Could not assign", value, "to a category");
/* no matching category */
// console.log("Could not assign", value, "to a category");
return unassigned_label;
};

Expand Down Expand Up @@ -62,8 +74,8 @@ export const computeMatrixFromRawData = (
) => {
/* color scale domain forms the categories in the stream graph */
const categories = colorScale.legendValues.filter((d) => d !== undefined);
categories.push(unassigned_label); /* for tips without a colorBy */
const isGenotype = isColorByGenotype(colorBy);

/* Matrix shape: first level (keys of Map) are values of the current colorBy. Type is preserved (hence the use of a Map())
second level (elements of array) are the pivots & their frequency value.
matrix.get('22L')[3] is the (3+1)th pivot for the color-by value 21L */
Expand All @@ -72,15 +84,14 @@ export const computeMatrixFromRawData = (
categories.forEach((x) => {
matrix.set(x, new Array(pivotsLen).fill(0));
});
matrix.set(unassigned_label, new Array(pivotsLen).fill(0)); // Will be removed later if no nodes assigned.

// let debugTipsSeen = 0;
const debugPivotTotals = new Array(pivotsLen).fill(0);
data.forEach((d) => {
if (visibility[d.idx] === NODE_VISIBLE) {
// debugTipsSeen++;
const category =
assignCategory(colorScale, categories, nodes[d.idx], colorBy, isGenotype) ||
unassigned_label;
// if (category === unassigned_label) return;
const category = assignCategory(colorScale, categories, nodes[d.idx], colorBy, isGenotype);
for (let i = 0; i < pivotsLen; i++) {
matrix.get(category)[i] += d.values[i];
debugPivotTotals[i] += d.values[i];
Expand Down
4 changes: 4 additions & 0 deletions src/util/tipRadiusHelpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { getTraitFromNode } from "../util/treeMiscHelpers";
* equates a single tip and a legend element
* exact match is required for categorical quantities such as genotypes, regions
* continuous variables need to fall into the interval (lower_bound, upper_bound]
* except for the first (smallest) legend value where we also match value=lower_bound
* @param {string|number} selectedLegendItem e.g. "USA" or 2021
* @param {object} node - node (tip) in question
* @param {object} colorScale - used to get the value of the attribute being used for colouring
Expand All @@ -14,6 +15,9 @@ import { getTraitFromNode } from "../util/treeMiscHelpers";
export const determineLegendMatch = (selectedLegendItem, node, colorScale) => {
const nodeAttr = getTipColorAttribute(node, colorScale);
if (colorScale.continuous) {
if (selectedLegendItem === colorScale.legendValues[0] && nodeAttr===colorScale.legendBounds[selectedLegendItem][0]) {
return true;
}
return (nodeAttr <= colorScale.legendBounds[selectedLegendItem][1]) &&
(nodeAttr > colorScale.legendBounds[selectedLegendItem][0]);
}
Expand Down