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

Gradient Coloring Option for Feature Metadata #484

Open
wants to merge 27 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
61f49b2
Add custom color
kwcantrell Feb 1, 2021
1f26583
checkpoint
kwcantrell Feb 10, 2021
dd2cfe6
added gradient coloring for feature metadata
kwcantrell Feb 10, 2021
20b369a
linter
kwcantrell Feb 10, 2021
6172f36
style fix
kwcantrell Feb 10, 2021
a3a1fe4
Show warning to user
kwcantrell Feb 22, 2021
3a69861
checkpoint
kwcantrell Feb 23, 2021
ca591ba
gradient feature metadata coloring
kwcantrell Feb 23, 2021
86ef3cf
fixed style issue
kwcantrell Feb 23, 2021
f2920c5
Merge branch 'master' of https://github.com/biocore/empress into grad…
kwcantrell Feb 24, 2021
a4052df
addressed comments
kwcantrell Mar 2, 2021
84d5b4a
style fix
kwcantrell Mar 2, 2021
8836260
Apply suggestions from code review
kwcantrell Mar 2, 2021
1431e2c
Merge branch 'master' of https://github.com/biocore/empress into grad…
kwcantrell Apr 9, 2021
743a32e
fix style
kwcantrell Apr 9, 2021
a90871c
fixed continuous error
kwcantrell Apr 9, 2021
305aa47
remove try-catch
kwcantrell Apr 12, 2021
d5426f6
Apply suggestions from code review
kwcantrell May 4, 2021
bf3f5fd
Merge branch 'master' of https://github.com/biocore/empress into grad…
kwcantrell Jun 10, 2021
aba2aa7
checkpoint
kwcantrell Jun 11, 2021
942f5b0
added test/fixed warning
kwcantrell Jun 11, 2021
9bd5961
fixed docs
kwcantrell Jun 11, 2021
6033e11
abstracted Legend class
kwcantrell Jun 13, 2021
1472ee4
Apply suggestions from code review
kwcantrell Aug 3, 2021
25ac9bd
address error 1
kwcantrell Aug 26, 2021
0add7c7
fixed conflicts
kwcantrell Aug 26, 2021
8206de8
fixed color bug
kwcantrell Aug 26, 2021
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
2 changes: 1 addition & 1 deletion empress/support_files/css/empress.css
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ p.side-header button:hover,
#legend-main {
margin: 20px;
min-width: 150px;
max-width: 33vw;
max-width: 30vw;
min-height: 30px;
max-height: 85vh;
padding-bottom: 0.1px;
Expand Down
91 changes: 76 additions & 15 deletions empress/support_files/js/empress.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,12 +158,26 @@ define([
this._treeData[i].splice(this._tdToInd.visible, 0, true);
}

/**
* @type {String}
* Text to display at the bottom of the continuous legend when some
* values in a continuous are either missing or non-numeric.
*/
this._continuousMissingNonNumericWarning =
"Some value(s) in this field were missing and/or not numeric. " +
"These value(s) have been left out of the gradient, and the " +
"corresponding nodes have been set to the default color.";
/**
* @type {Legend}
* Legend describing the way the tree is colored.
* @private
*/
this._legend = new Legend(document.getElementById("legend-main"));
this._legend.setMissingNonNumericWarning(
"Some value(s) in this field were missing and/or not numeric. " +
"These value(s) are not included in the gradient, and the " +
"associated nodes have been left as the default color."
);
kwcantrell marked this conversation as resolved.
Show resolved Hide resolved

/**
* @type {BiomTable}
Expand Down Expand Up @@ -2443,14 +2457,19 @@ define([
* @param{Boolean} reverse Defaults to false. If true, the color scale
* will be reversed, with respect to its default
* orientation.
*
* @param{Boolean} continuous Defaults to false. If true, the colorer will
* try to use a gradient color scale.
* @param{Function} continousFailedFunc The function to call if continuous
* coloring failed.
* @return {Object} Maps unique values in this f. metadata column to colors
*/
Empress.prototype.colorByFeatureMetadata = function (
cat,
color,
method,
reverse = false
reverse = false,
continuous = false,
continousFailedFunc = null
fedarko marked this conversation as resolved.
Show resolved Hide resolved
) {
var fmInfo = this.getUniqueFeatureMetadataInfo(cat, method);
var sortedUniqueValues = fmInfo.sortedUniqueValues;
Expand All @@ -2464,18 +2483,57 @@ define([
obs[uniqueVal] = new Set(uniqueValueToFeatures[uniqueVal]);
});

// assign colors to unique values
var colorer = new Colorer(
color,
sortedUniqueValues,
undefined,
undefined,
reverse
);
var colorer;
try {
// assign colors to unique values
colorer = new Colorer(
color,
sortedUniqueValues,
continuous,
// Colorer will create a special gradient ID using the number
// we pass into this parameter. This allows empress to display
// multiple gradients at the same time without them overriding
// each other. Currently, the barplots are set up to start at
// 0. So, we set this value to -1 here to avoid conflict with
// the barplot gradients; this allows us to display the
// feature metadata gradient alongside the
// barplot gradients.
continuous ? -1 : undefined,
reverse
);
} catch (err) {
// If the Colorer construction failed (should only have
// happened if the user asked for continuous values but the
// selected field doesn't have at least 2 unique numeric
// values), then we open a toast message about this error and
// use discrete coloring instead.
continuous = false;
var msg =
'Error with assigning colors: the feature metadata field "' +
cat +
'" has less than 2 unique numeric values, so it cannot be ' +
"used for continuous coloring. " +
"Using discrete coloring instead.";
util.toastMsg(msg, 5000);
kwcantrell marked this conversation as resolved.
Show resolved Hide resolved
// assign colors to unique values
colorer = new Colorer(
color,
sortedUniqueValues,
continuous,
undefined,
reverse
);
continousFailedFunc();
}
// colors for drawing the tree
var cm = colorer.getMapRGB();
// colors for the legend
var keyInfo = colorer.getMapHex();
var keyInfo;
if (continuous) {
keyInfo = colorer.getGradientInfo();
} else {
keyInfo = colorer.getMapHex();
}

// Do upwards propagation only if the coloring method is "tip"
if (method === "tip") {
Expand All @@ -2487,8 +2545,11 @@ define([

// color tree
this._colorTree(obs, cm);

this.updateLegendCategorical(cat, keyInfo);
if (continuous) {
this._legend.addContinuousKey(cat, keyInfo);
kwcantrell marked this conversation as resolved.
Show resolved Hide resolved
kwcantrell marked this conversation as resolved.
Show resolved Hide resolved
} else {
fedarko marked this conversation as resolved.
Show resolved Hide resolved
this.updateLegendCategorical(cat, keyInfo);
}

return keyInfo;
};
Expand All @@ -2502,7 +2563,7 @@ define([
*
* 2) Assigns each internal node to a group if all of its children belong
* to the same group.
*@t
*
* 3) Remove empty groups from return object.
*
* Note: All tips that are not passed into obs are considered to belong to
Expand Down Expand Up @@ -2969,7 +3030,7 @@ define([
// project groups up tree
// Note: if _projectObservations was called, then if an internal node
// belongs to a group, all of its descendants will belong to the
// same group. However, this is not guaranteed if _projectOBservations
// same group. However, this is not guaranteed if _projectObservations
// was not called. Thus, this loop is used to guarantee that if an
// internal node belongs to a group then all of its descendants belong
// to the same group.
Expand Down
32 changes: 28 additions & 4 deletions empress/support_files/js/legend.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,14 +179,15 @@ define(["jquery", "underscore", "util"], function ($, _, util) {
"svg"
);
containerSVG.setAttribute("width", "100%");
containerSVG.setAttribute("height", "100%");
containerSVG.setAttribute("height", "80%");
kwcantrell marked this conversation as resolved.
Show resolved Hide resolved
containerSVG.setAttribute("style", "display: block; margin: auto;");
// just kinda plop the combined SVG code into containerSVG's HTML
containerSVG.innerHTML = totalHTMLSVG;
this._container.appendChild(containerSVG);
if (this._missingNonNumericWarningShown) {
var missingText = this.getMissingNonNumericWarning();
var warningP = document.createElement("p");
warningP.innerText = Legend.CONTINUOUS_MISSING_NON_NUMERIC_WARNING;
warningP.innerText = missingText.full;
warningP.classList.add("side-panel-notes");
// All legends have white-space: nowrap; set to prevent color
// labels from breaking onto the next line (which would look
Expand Down Expand Up @@ -542,6 +543,7 @@ define(["jquery", "underscore", "util"], function ($, _, util) {
// But first, let's add a warning about missing / non-numeric values if
// needed.
if (this._missingNonNumericWarningShown) {
var missingText = this.getMissingNonNumericWarning();
// We use a hanging baseline to add some extra vertical space
// between the gradient minimum value and the warning text. This
// seems to look nice.
Expand All @@ -551,9 +553,9 @@ define(["jquery", "underscore", "util"], function ($, _, util) {
'" y="' +
(gradientTopY + gradientHeight + Legend.HALF_LINE_HEIGHT) +
'" dominant-baseline="hanging">' +
Legend.CONTINUOUS_MISSING_NON_NUMERIC_WARNING_SHORT +
missingText.short +
"</text>\n";
texts.push(Legend.CONTINUOUS_MISSING_NON_NUMERIC_WARNING_SHORT);
texts.push(missingText.short);
}
_.each(texts, function (text) {
maxLineWidth = Math.max(
Expand Down Expand Up @@ -742,6 +744,28 @@ define(["jquery", "underscore", "util"], function ($, _, util) {
};
};

Legend.prototype.setMissingNonNumericWarning = function (
Copy link
Collaborator

Choose a reason for hiding this comment

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

This works -- and I'm OK with merging it in as is, as long as the other changes I've suggested here are addressed -- but I think we could probably structure this code more clearly. To me, it is confusing that the Legend defaults to the barplot messages, and that these messages have to be passed in from Empress object (but the barplot messages are already located here).

I propose we restructure this so that the Legend class (on creation) takes as input an optional legendUse parameter of tree or barplot, which then adjusts what the full and short missing non-numeric warnings are (so all of the warning text will live in legend.js). That said: I'm totally OK with deferring this to a future issue so we can get this PR in soon.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree, this was a fairly lazy solution. The goal was to modify the existing code as little as possible hence why it defaults to the barplot messages.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Instead of adding a special parameter, I will make Legend an abstract class and add SampleFeatureColoringLegend and BarplotLegend that will inherit it. This will also make it a lot easier to change the behavior of the different legend types.

full,
short = null
) {
this.continuousMissingNonNumericWarning = full;
this.continuousMissingNonNumericWarningShort = short;
};

Legend.prototype.getMissingNonNumericWarning = function () {
var missingText = {
full: Legend.CONTINUOUS_MISSING_NON_NUMERIC_WARNING,
short: Legend.CONTINUOUS_MISSING_NON_NUMERIC_WARNING_SHORT,
};
if (this.hasOwnProperty("continuousMissingNonNumericWarning")) {
missingText.full = this.continuousMissingNonNumericWarning;
}
if (this.hasOwnProperty("continuousMissingNonNumericWarningShort")) {
missingText.short = this.continuousMissingNonNumericWarningShort;
}
return missingText;
};

// Shown at the bottom of continuous legends in the page when some values
// in a continuous field can't be represented on a gradient
Legend.CONTINUOUS_MISSING_NON_NUMERIC_WARNING =
Expand Down
20 changes: 19 additions & 1 deletion empress/support_files/js/side-panel-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ define(["underscore", "Colorer", "util"], function (_, Colorer, util) {
this.fCollapseCladesChk = document.getElementById(
"feature-collapse-chk"
);
this.fContinuousDiv = document.getElementById("feature-continous-div");
this.fContinuousChk = document.getElementById(
"feature-continuous-color-chk"
);
this.fLineWidth = document.getElementById("feature-line-width");
this.fUpdateBtn = document.getElementById("feature-update");
this.fMethodChk = document.getElementById("fm-method-chk");
Expand Down Expand Up @@ -330,15 +334,23 @@ define(["underscore", "Colorer", "util"], function (_, Colorer, util) {
* Colors the tree based on the feature metadata coloring settings.
*/
SidePanel.prototype._colorFeatureTree = function () {
var scope = this;
var colBy = this.fSel.value;
var col = this.fColor.value;
var coloringMethod = this.fMethodChk.checked ? "tip" : "all";
var reverse = this.fReverseColor.checked;
var continuous =
!this.fContinuousDiv.classList.contains("hidden") &&
this.fContinuousChk.checked;
this.empress.colorByFeatureMetadata(
fedarko marked this conversation as resolved.
Show resolved Hide resolved
colBy,
col,
coloringMethod,
reverse
reverse,
continuous,
() => {
scope.fContinuousChk.checked = false;
}
);
};

Expand Down Expand Up @@ -608,10 +620,16 @@ define(["underscore", "Colorer", "util"], function (_, Colorer, util) {
};

var showUpdateBtn = function () {
if (Colorer.isColorMapDiscrete(scope.fColor.value)) {
scope.fContinuousDiv.classList.add("hidden");
} else {
scope.fContinuousDiv.classList.remove("hidden");
}
scope.fUpdateBtn.classList.remove("hidden");
};
this.fSel.onchange = showUpdateBtn;
this.fColor.onchange = showUpdateBtn;
this.fContinuousChk.onchange = showUpdateBtn;
this.fReverseColor.onchange = showUpdateBtn;
this.fLineWidth.onchange = showUpdateBtn;
this.fMethodChk.onchange = function () {
Expand Down
7 changes: 7 additions & 0 deletions empress/support_files/templates/side-panel.html
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,13 @@
If checked, collapse all clades whose members share the same feature
metadata value.
</p>
<div id="feature-continous-div" class="hidden">
<p>
<label for="feature-continuous-color-chk">Continuous values?</label>
<input id="feature-continuous-color-chk" type="checkbox" value="false"
class="empress-input">
</p>
</div>
fedarko marked this conversation as resolved.
Show resolved Hide resolved
<p>
<label for="feature-line-width">Extra Line Width</label>
<input id="feature-line-width" type="number" value="0" min="0"
Expand Down
Loading