Fix saliency viewing in example viewer #1472
Conversation
jameswex
commented
Sep 28, 2018
jameswex
commented
- Fix issues for vz_example_viewer saliency viewing introduced with WIT changes
- Sort features by saliency when it is provided
- Visual change to saliency legend to match previous visual changes to overall example viewer
| haveSaliency: function() { | ||
| if (!this.featuresList || !this.saliency || | ||
| if (!this.filteredFeaturesList || !this.saliency || | ||
| Object.keys(this.saliency).length === 0 || !this.colors) { | ||
| return; | ||
| } | ||
|
|
||
| // TODO(jwexler): Find a way to do this without requestAnimationFrame. |
There was a problem hiding this comment.
Just saying, this seems to happen because of _debounceTemplate in dom-repeat: https://github.com/Polymer/polymer/blob/1.x/src/lib/template/dom-repeat.html#L492
| .style('background', 'lightgrey'); | ||
|
|
||
| this.selectAll('.value-pill') | ||
| .style('background', '#e8eaed'); |
There was a problem hiding this comment.
I am not sure how this code relates to the comment in L373. Are they related? Also, would it be possible to hoist this constant color value as a module level constant (i.e., neutralSaliencyColor)?
There was a problem hiding this comment.
fixed comment and changed to use neutralSaliencyColor as it should
| const moreButton = this.selectAll( | ||
| `paper-button.${this.sanitizeFeature(feat.name)}.value-pill`); | ||
| let mostExtremeSal = 0; | ||
| for (let i = 1; i < valArray.length; i++) { |
There was a problem hiding this comment.
no action required: an alternative to write this is, Math.max(...valArray.map(Math.abs))
There was a problem hiding this comment.
Did this way so as to keep the most extreme true value from valArray instead of the abs value of it.
| @@ -326,29 +349,33 @@ Polymer({ | |||
| ]); | |||
| }, | |||
|
|
|||
| selectAll: function(query: string) { | |||
| return d3.selectAll( | |||
| Polymer.dom(this.root).querySelectorAll(query) as any); | |||
There was a problem hiding this comment.
Just wondering, how does this differ from this.querySelectorAll?
There was a problem hiding this comment.
probably no different, was just following what i saw in the doc at https://www.polymer-project.org/1.0/docs/devguide/local-dom
| // its feature values, to sort features with the most salienct features at | ||
| // the top. | ||
| const saliencyTotals = checkSal ? | ||
| Object.assign({}, ...Object.keys(saliency).map( |
There was a problem hiding this comment.
This is doing Object.assign on objects that you map out of keys of saliency. I think you can do this either by cloning saliency object and transforming its values or
Object.entries(saliency)
.map(([key, value]) => [key, typeof value == 'number' ? [value] : value])
.map(([key, value]) => [key, value.map(Math.abs).reduce((total, cur) => total + cur, 0)])
.reduce((obj, [key, value]) => {
obj[key] = value;
return obj;
}, {})
though I am not sure if this has any better readability.
