-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Update quiver trace API #7945
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
base: v4.0
Are you sure you want to change the base?
Update quiver trace API #7945
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - Rename `quiver` trace attributes for clarity and consistency: `sizeref` -> `lengthfactor`, `sizemode` -> `lengthmode`, `anglemode` -> `uvref` [[#7945](https://github.com/plotly/plotly.js/issues/7945)] | ||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -33,8 +33,8 @@ module.exports = function calc(gd, trace) { | |||||||||
| const uArr = trace.u || []; | ||||||||||
| const vArr = trace.v || []; | ||||||||||
|
|
||||||||||
| const anglemode = trace.anglemode; | ||||||||||
| const sizemode = trace.sizemode; | ||||||||||
| const uvref = trace.uvref; | ||||||||||
| const lengthmode = trace.lengthmode; | ||||||||||
| const anchor = trace.anchor; | ||||||||||
|
Comment on lines
+36
to
38
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| const isTip = anchor === 'tip'; | ||||||||||
| const isCenter = anchor === 'center'; | ||||||||||
|
|
@@ -54,7 +54,7 @@ module.exports = function calc(gd, trace) { | |||||||||
| var nValid = 0; | ||||||||||
|
|
||||||||||
| // First pass: build calcdata, and keep track of the maximum and minimum vector norm in the trace, | ||||||||||
| // to be used for sizemode 'scaled' (max norm only) and for magnitude-based colorscale range | ||||||||||
| // to be used for lengthmode 'scaled' (max norm only) and for magnitude-based colorscale range | ||||||||||
| for(var i = 0; i < len; i++) { | ||||||||||
| var cdi = cd[i] = { i: i }; | ||||||||||
| var xValid = isNumeric(xVals[i]); | ||||||||||
|
|
@@ -109,19 +109,19 @@ module.exports = function calc(gd, trace) { | |||||||||
| // Store maxNorm for use by plot step | ||||||||||
| trace._maxNorm = normMax; | ||||||||||
|
|
||||||||||
| if (sizemode === 'scaled' || anglemode === 'paper') { | ||||||||||
| // Ignore sizemode 'raw' if anglemode is set to 'paper': always scale | ||||||||||
| if (lengthmode === 'scaled' || uvref === 'paper') { | ||||||||||
| // Ignore lengthmode 'raw' if uvref is set to 'paper': always scale | ||||||||||
|
|
||||||||||
| // Compute point density of the entire trace: Area of bounding box | ||||||||||
| // divided by number of points. This is used to scale arrows in | ||||||||||
| // 'scaled' sizemode. | ||||||||||
| // 'scaled' lengthmode. | ||||||||||
| // TODO: How to handle the case where there is just one point in a trace, | ||||||||||
| // or all points have the same x or y value? This will give a boxArea of 0. | ||||||||||
| // For now I'm going to just normalize to a vector of unit length (1) in that case, | ||||||||||
| // but that's not a great solution | ||||||||||
|
Comment on lines
118
to
121
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this comment be addressed? |
||||||||||
| const boxArea = (xMax - xMin) * (yMax - yMin); | ||||||||||
| const pointDensity = boxArea / len; | ||||||||||
| // Now, compute the scale factor for scaled size mode | ||||||||||
| // Now, compute the scale factor for scaled length mode | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| // The scale factor should be such that | ||||||||||
| // _maxNorm * _scaleFactor = Math.sqrt(_pointDensity) | ||||||||||
| // Therefore: _scaleFactor = Math.sqrt(_pointDensity) / _maxNorm | ||||||||||
|
|
@@ -130,16 +130,16 @@ module.exports = function calc(gd, trace) { | |||||||||
| } else { | ||||||||||
| trace._scaleFactor = Math.sqrt(pointDensity) / trace._maxNorm; | ||||||||||
| } | ||||||||||
| // Note: If anglemode === 'paper', this scale factor must be | ||||||||||
| // Note: If uvref === 'paper', this scale factor must be | ||||||||||
| // multiplied by Math.sqrt(xa._m * ya._m), but we can't do that quite yet | ||||||||||
| // since the axis scales are not fully determined. Do it in plot step instead. | ||||||||||
| } else { // sizemode === 'raw' | ||||||||||
| // For raw sizemode, scale factor is always 1 | ||||||||||
| } else { // lengthmode === 'raw' | ||||||||||
| // For raw lengthmode, scale factor is always 1 | ||||||||||
|
Comment on lines
+136
to
+137
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO, the fact that it's always 1 is evident from the code.
Suggested change
|
||||||||||
| trace._scaleFactor = 1; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Multiply scale factor by sizeref | ||||||||||
| trace._scaleFactor *= trace.sizeref; | ||||||||||
| // Multiply computed scale factor by lengthfactor attr | ||||||||||
| trace._scaleFactor *= trace.lengthfactor; | ||||||||||
|
|
||||||||||
| // Now we need to compute the arrow geometry for axis autorange | ||||||||||
| const xTipPositions = new Array(len); | ||||||||||
|
|
@@ -148,7 +148,7 @@ module.exports = function calc(gd, trace) { | |||||||||
| const yTailPositions = new Array(len); | ||||||||||
| var arrowLenX, arrowLenY; | ||||||||||
| // Compute the x- and y-positions of the tip of each arrow, | ||||||||||
| // assuming anglemode === 'data' (i.e. u/v are in data coordinates) | ||||||||||
| // assuming uvref === 'data' (i.e. u/v are in data coordinates) | ||||||||||
| for(var i = 0; i < len; i++) { | ||||||||||
| var cdi = cd[i]; | ||||||||||
| arrowLenX = cdi._u * trace._scaleFactor; | ||||||||||
|
|
@@ -171,12 +171,12 @@ module.exports = function calc(gd, trace) { | |||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if (anglemode === 'data') { | ||||||||||
| // If anglemode is 'data', we can use the arrow tip positions directly to expand the axes ranges | ||||||||||
| if (uvref === 'data') { | ||||||||||
| // If uvref is 'data', we can use the arrow tip positions directly to expand the axes ranges | ||||||||||
| trace._extremes[xa._id] = Axes.findExtremes(xa, xTipPositions.concat(xTailPositions), {padded: true}); | ||||||||||
| trace._extremes[ya._id] = Axes.findExtremes(ya, yTipPositions.concat(yTailPositions), {padded: true}); | ||||||||||
| } else { // anglemode === 'paper' | ||||||||||
| // TODO: For now, just do the same thing as for anglemode === 'data', but this is not correct. | ||||||||||
| } else { // uvref === 'paper' | ||||||||||
| // TODO: For now, just do the same thing as for uvref === 'data', but this is not correct. | ||||||||||
| // We actually need more sophisticated logic here, since this will give a bad result | ||||||||||
| // if the data aspect ratio is very different from the plot aspect ratio. | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a follow up issue to track this? |
||||||||||
| trace._extremes[xa._id] = Axes.findExtremes(xa, xTipPositions.concat(xTailPositions), {padded: true}); | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -103,13 +103,11 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition | |||||
|
|
||||||
| // Use maxNorm precomputed in calc | ||||||
| const maxNorm = trace._maxNorm || 0; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| const anglemode = trace.anglemode; | ||||||
| const sizemode = trace.sizemode; | ||||||
| const sizeref = trace.sizeref; | ||||||
| const uvref = trace.uvref; | ||||||
| const anchor = trace.anchor; | ||||||
|
Comment on lines
+106
to
107
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| // Adjust scale factor if anglemode is 'paper' | ||||||
| const scaleFactor = (anglemode === 'paper') ? trace._scaleFactor * Math.sqrt(Math.abs(xa._m * ya._m)) : trace._scaleFactor; | ||||||
| // Adjust scale factor if uvref is 'paper' | ||||||
| const scaleFactor = (uvref === 'paper') ? trace._scaleFactor * Math.sqrt(Math.abs(xa._m * ya._m)) : trace._scaleFactor; | ||||||
| const markerArrowsize = trace.marker.arrowsize; | ||||||
|
|
||||||
| // Update line segments | ||||||
|
|
@@ -123,12 +121,12 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition | |||||
| } | ||||||
|
|
||||||
| // Compute pixel location of vector tip, *relative to* vector base (before scaling). | ||||||
| // If anglemode is 'paper', then u/v are interpreted in pixel coordinates, so we can use them directly. | ||||||
| // If anglemode is 'data', then u/v are interpreted in data coordinates, so we need to convert them to pixel coordinates. | ||||||
| // If uvref is 'paper', then u/v are interpreted in pixel coordinates, so we can use them directly. | ||||||
| // If uvref is 'data', then u/v are interpreted in data coordinates, so we need to convert them to pixel coordinates. | ||||||
| // TODO: This probably doesn't work for log axes, but let's ignore log axes for now | ||||||
| // since I'm not sure they make sense for quiver plots anyway | ||||||
| const pu = ((anglemode === 'paper') ? cdi._u * Math.sign(xa._m) : d3.round(xa._m * cdi._u)) * scaleFactor; | ||||||
| const pv = ((anglemode === 'paper') ? cdi._v * Math.sign(ya._m) : d3.round(ya._m * cdi._v)) * scaleFactor; | ||||||
| const pu = ((uvref === 'paper') ? cdi._u * Math.sign(xa._m) : d3.round(xa._m * cdi._u)) * scaleFactor; | ||||||
| const pv = ((uvref === 'paper') ? cdi._v * Math.sign(ya._m) : d3.round(ya._m * cdi._v)) * scaleFactor; | ||||||
|
|
||||||
| // Compute arrow in data space | ||||||
| // Check whether arrowsize was set explicitly in the input trace | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @emilykl,
Since this feature is not released yet, I suggest you modify
7710_add.mdlog instead and mention this PR.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with this suggestion.