-
-
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
Open
emilykl
wants to merge
8
commits into
v4.0
Choose a base branch
from
update-quiver-api
base: v4.0
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Update quiver trace API #7945
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
5ab9743
update attribute names in src/traces/quiver/
emilykl 003062c
regenerate types and plot-schema
emilykl d3d7046
update jasmine tests
emilykl 00ba1a5
update image mocks and baselines
emilykl 7c9f063
add draftlog
emilykl a3c5fa2
Apply suggestions from code review
emilykl d7a39f7
rename uvref to arrowref
emilykl b46f29c
combine draftlogs
emilykl File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| - Add `quiver` trace type to visualize vector fields using arrows [[#7710](https://github.com/plotly/plotly.js/pull/7710)], with thanks to @degzhaus for the contribution! | ||
| - Add `quiver` trace type to visualize vector fields using arrows [[#7710](https://github.com/plotly/plotly.js/pull/7710), [#7945](https://github.com/plotly/plotly.js/issues/7945)], with thanks to @degzhaus for the contribution! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,9 +33,7 @@ module.exports = function calc(gd, trace) { | |
| const uArr = trace.u || []; | ||
| const vArr = trace.v || []; | ||
|
|
||
| const anglemode = trace.anglemode; | ||
| const sizemode = trace.sizemode; | ||
| const anchor = trace.anchor; | ||
| const { anchor, lengthmode, arrowref } = trace; | ||
| const isTip = anchor === 'tip'; | ||
| const isCenter = anchor === 'center'; | ||
|
|
||
|
|
@@ -54,7 +52,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 +107,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' || arrowref === 'paper') { | ||
| // Ignore lengthmode 'raw' if arrowref 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 | ||
| 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 lengthmode | ||
| // The scale factor should be such that | ||
| // _maxNorm * _scaleFactor = Math.sqrt(_pointDensity) | ||
| // Therefore: _scaleFactor = Math.sqrt(_pointDensity) / _maxNorm | ||
|
|
@@ -130,16 +128,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 arrowref === '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' | ||
| 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 +146,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 arrowref === '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 +169,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 (arrowref === 'data') { | ||
| // If arrowref 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 { // arrowref === 'paper' | ||
| // TODO: For now, just do the same thing as for arrowref === '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}); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Can this comment be addressed?