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

ENH Feature metadata coloring command pattern #440

Closed
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
313 changes: 313 additions & 0 deletions empress/support_files/js/commands.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,313 @@
define(["underscore"], function (_) {
/**
*
* @class CommandManager
*
* Maintains and provides operations for manipulating a FIFO queue of commands.
*
* @param {Empress} empress The empress object to execute all of the commands on.
*
* @return {CommandManager}
* @constructs CommandManager
*/
function CommandManager(empress) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not all that urgent, but it would be good to add a comment here similar to what the Command class has below (just to make it clear that this is also a "class", at least kinda). There are examples of this (for non-explicitly-declared JS classes in Empress) in bp-tree.js and empress.js.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added in 00baae9

this.empress = empress;
this.executed = [];

Choose a reason for hiding this comment

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

This queue is a little weird. If the design involves some external message bus, I would think that message bus would keep track of all commands that pass through it, or even some specific logger component. I don't see a value in having every instance of empress maintain commands that it executed.

Choose a reason for hiding this comment

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

One use case this one makes sense for is Undo/Redo functionality, but I don't think that really works with a message bus architecture and I haven't heard it being requested. (It also requires all commands to store enough state to be invertible)

Copy link
Member Author

Choose a reason for hiding this comment

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

Right so this CommandManager is intended just to queue up commands for empress only, and hopefully be able to replay some actions on empress. We would require a separate class to manage interactions with an event bus.

this.toExecute = [];
}

/**
* Adds a new command at the end of the queue
*
* @param {Command} command Command to be added to the queue.
*/
CommandManager.prototype.push = function (command) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess now that we're at the stage of getting ready to merge this in, it'd be nice to add some basic docs for these functions (sorry, I realize this is going to be like 4 separate comments ._.)

At the very least, it'd be good to be very clear that a first-in first-out queue is used (... at least that is what I think is done from reading this).

Copy link
Member Author

Choose a reason for hiding this comment

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

Added in 00baae9

this.toExecute.push(command);
};

/**
* Removes the first Command from the queue and executes it.
*/
CommandManager.prototype.executeNext = function () {
var command = this.toExecute.shift();
command.execute(this.empress);
this.executed.push(command);
};

/**
* Executes all commands that have yet to be executed.
*/
CommandManager.prototype.executeAll = function () {
while (this.toExecute.length > 0) {
this.executeNext();
}
};

/**
* Adds a command to the queue then executes everything that has yet
* to be executed.
*
* @param {Command} command Command to be executed.
*/
CommandManager.prototype.pushAndExecuteAll = function (command) {
this.toExecute.push(command);
this.executeAll();
};

/**
* Encapsulates the information needed to perform an action.
* @class Command
*/
class Command {
/**
* Validates props and then assigns them to this object.
*
* @param {Object} props Contains the properties needed to perform the action.
* Defaults to empty object
*/
constructor(props = {}) {
this.constructor.validateProps(props);
Object.assign(this, props);
}

/**
* Validates the properties passed to the constructor. Should throw
* an error if there is a violation.
*
* @param props {Object} props Properties that need to be validated
*/
static validateProps(props) {}

/**
* Executes the action specified by the command.
* @param controller An object that is needed at runtime for the command.
*/
execute(controller) {}
}

/**
* Initializes and executes a command.
*
* @param {Class} commandClass The class of the command to instantiate
* @param {Object} props Contains the properties needed to perform the action.
* @param controller An object that is need at runtime for the command.
*/
function initAndExecute(commandClass, props, controller) {
var command = new commandClass(props);
command.execute(controller);
}

/**
* Checks props for required keys. Throws an error if a required key is missing.
*
* @param {Object} props The properties to check.
* @param {Array} required The keys that are required in props.
*/
function checkRequired(props, required) {
required.forEach(function (name) {
if (!(name in props)) {
throw "Required: '" + name + "' not in props";
}
});
}

/**
* A command that performs no validation and does nothing in its execute call.
*
* @class NullCommand
*/
class NullCommand extends Command {
/**
* Performs no validation.
* @param {Object} props
*/
static validateProps(props) {}

/**
* Performs no action.
* @param controller
*/
execute(controller) {}
}

/**
* Sets the color of the empress tree back to default.
*/
class ResetTreeCommand extends Command {
execute(empress) {
empress.resetTree();
}
}

var requiredColorProps = ["colorBy", "colorMapName", "coloringMethod"];

/**
* Colors the tree by feature metadata
*
* @class ColorByFeatureMetadataCommand
*
*/
class ColorByFeatureMetadataCommand extends Command {
/**
* @param {Object} props Properties to use for coloring based on feature metadata
* @param {string} props.colorBy Category to color based on.
* @param {string} props.colorMapName Name of color map to use.
* @param {string} props.coloringMethod Method to use for coloring.
* @param {Boolean} props.reverseColorMap (optional) Indicates whether the the color
* map should be reverse.
*/
constructor(props) {
super(props);
}

/**
* Ensures props contain enough information to execute the command.
*
* @param {Object} props passed to this objects constructor.
*/
static validateProps(props) {
super.validateProps(props);
checkRequired(props, requiredColorProps);
}

/**
* Colors the empress object by feature metadata
*
* @param {Empress} empress The empress object to color by feature metadata
*/
execute(empress) {
empress.colorByFeatureMetadata(
this.colorBy,
this.colorMapName,
this.coloringMethod,
this.reverseColorMap
);
}
}

/**
* Collapses the clades of an empress object.
*/
class CollapseCladesCommand extends Command {
/**
* Executes the clade collapse
* @param {Empress} empress The empress object to collapse.
*/
execute(empress) {
empress.collapseClades();
}
}

var requiredThickenProps = ["lineWidth"];

/**
* Thickens the colored branches of the empress object
*/
class ThickenColoredNodesCommand extends Command {
/**
*
* @param {Object} props Properties to use for thickening the branches
* @param {Number} props.lineWidth Amount of thickness to use.
*/
constructor(props) {
super(props);
}

/**
*
* @param {Object} props Properties to validate.
*/
static validateProps(props) {
super.validateProps(props);
checkRequired(props, requiredThickenProps);
}

/**
* Executes the node thickening.
*
* @param {Empress} empress The Empress object to thicken the nodes of.
*/
execute(empress) {
empress.thickenColoredNodes(this.lineWidth);
}
}

/**
* Command for drawing the tree.
*/
class DrawTreeCommand extends Command {
/**
* Executes the tree drawing.
* @param {Empress} empress The Empress object to draw the tree of.
*/
execute(empress) {
empress.drawTree();
}
}

/**
* A convenience class for chaining commands, that is a Command itself.
*/
class CompositeCommand extends Command {
constructor(props) {
super(props);
this.commands = [];
}

execute(controller) {
this.commands.forEach(function (command) {
command.execute(controller);
});
}
}

/**
* Composite pipeline for performing multiple steps of coloring a tree by feature metadata
*/
class ColorByFeatureMetadataPipeline extends CompositeCommand {
/**
*
* @param {Object} props Properties used to color the tree by metadata.
* @param {string} props.colorBy Category to color based on.
* @param {string} props.colorMapName Name of color map to use.
* @param {string} props.coloringMethod Method to use for coloring.
* @param {Number} props.lineWidth Amount of thickness to use.
* @param {Boolean} props.reverseColorMap (optional) Indicates whether the the color
* map should be reverse.
*/
constructor(props) {
super(props);
var reset = new ResetTreeCommand();
var color = new ColorByFeatureMetadataCommand({
colorBy: this.colorBy,
colorMapName: this.colorMapName,
coloringMethod: this.coloringMethod,
reverseColorMap: this.reverseColorMap,
});
var collapse;
if (this.collapseClades) {
collapse = new CollapseCladesCommand();
} else {
// Command is the null command
collapse = new NullCommand();
}
var thicken = new ThickenColoredNodesCommand({
lineWidth: this.lineWidth,
});
var draw = new DrawTreeCommand();

this.commands = [reset, color, collapse, thicken, draw];
}
}

return {
CollapseCladesCommand: CollapseCladesCommand,
ColorByFeatureMetadataCommand: ColorByFeatureMetadataCommand,
ColorByFeatureMetadataPipeline: ColorByFeatureMetadataPipeline,
CommandManager: CommandManager,
DrawTreeCommand: DrawTreeCommand,
NullCommand: NullCommand,
ResetTreeCommand: ResetTreeCommand,
ThickenColoredNodesCommand: ThickenColoredNodesCommand,
};
});
34 changes: 27 additions & 7 deletions empress/support_files/js/side-panel-handler.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
define(["underscore", "Colorer", "util"], function (_, Colorer, util) {
define(["underscore", "Colorer", "util", "Commands"], function (
_,
Colorer,
util,
Commands
) {
/**
*
* @class SidePanel
Expand Down Expand Up @@ -29,6 +34,7 @@ define(["underscore", "Colorer", "util"], function (_, Colorer, util) {

// used to event triggers
this.empress = empress;
this.commandManager = new Commands.CommandManager(this.empress);

// settings components
this.treeNodesChk = document.getElementById("display-nodes-chk");
Expand Down Expand Up @@ -564,12 +570,26 @@ define(["underscore", "Colorer", "util"], function (_, Colorer, util) {
};

this.fUpdateBtn.onclick = function () {
scope._updateColoring(
"_colorFeatureTree",
scope.fCollapseCladesChk,
scope.fLineWidth,
scope.fUpdateBtn
);
// hide update button
scope.fUpdateBtn.classList.add("hidden");

// this came from colorMethodName
var colBy = scope.fSel.value;
var col = scope.fColor.value;
var coloringMethod = scope.fMethodChk.checked ? "tip" : "all";
var reverse = scope.fReverseColor.checked;
var collapseChecked = scope.fCollapseCladesChk.checked;
var lw = util.parseAndValidateNum(scope.fLineWidth);

var command = new Commands.ColorByFeatureMetadataPipeline({
collapseClades: collapseChecked,
lineWidth: lw,
colorBy: colBy,
colorMapName: col,
coloringMethod: coloringMethod,
reverseColorMap: reverse,
});
scope.commandManager.pushAndExecuteAll(command);
};

this.fCollapseCladesChk.onclick = function () {
Expand Down
5 changes: 3 additions & 2 deletions empress/support_files/templates/empress-template.html
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ <h3 class="hidden" id="menu-sm-header">Sample Presence Information</h3>
'ByteArray' : './js/byte-array',
'BPTree' : './js/bp-tree',
'Camera' : './js/camera',
'Commands': './js/commands',
'Drawer' : './js/drawer',
'SidePanel' : './js/side-panel-handler',
'AnimationPanel' : './js/animation-panel-handler',
Expand All @@ -114,12 +115,12 @@ <h3 class="hidden" id="menu-sm-header">Sample Presence Information</h3>
'SidePanel', 'AnimationPanel', 'Animator', 'BarplotLayer',
'BarplotPanel', 'BIOMTable', 'Empress', 'Legend',
'Colorer', 'VectorOps', 'CanvasEvents', 'SelectedNodeMenu',
'util', 'LayoutsUtil', 'ExportUtil'],
'util', 'LayoutsUtil', 'ExportUtil', 'Commands'],
function($, gl, chroma, underscore, spectrum, filesaver, ByteArray,
BPTree, Camera, Drawer, SidePanel, AnimationPanel, Animator,
BarplotLayer, BarplotPanel, BIOMTable, Empress, Legend,
Colorer, VectorOps, CanvasEvents, SelectedNodeMenu, util,
LayoutsUtil, ExportUtil) {
LayoutsUtil, ExportUtil, Commands) {
// initialze the tree and model
var tree = new BPTree(
{{ tree }},
Expand Down
Loading