Add quiver trace type for vector field visualization#7710
Add quiver trace type for vector field visualization#7710degzhaus wants to merge 35 commits intoplotly:masterfrom
Conversation
1262c1e to
f327f5d
Compare
|
Thanks again for the great comments, @emilykl ! I will attempt to reference commits inline from this comment here: -- Bug Fixes -- "The axis ranges don't automatically adjust to include the full length of the arrows" fixed here: "Colorscale seems broken in the latest version" fixed here: "Legends are missing icons" fixed here: " "The "Display of colorbars does not seem to work" fixed here: -- API Refinements -- " "The line options should be nested under marker.line", |
|
Here are the followups for the remaining 3 comments from: #7584, @emilykl : "Could we plausibly reuse the src/traces/scatter/select.js selectPoints() function" fixed here: "Should probably call the hasColorscale() function here" fixed here: "Can you look into whether we can/should use the handleXYDefaults() function here" fixed here: |
|
Alrighty, @emilykl , thanks again for your great feedback and patience as I addressed it! I have pushed all my fixes and tested locally, I think we're close 🤞 |
|
Great news @degzhaus ! Thank you for the continued work and the clear summary. I will try to take another look within the next week. Feel free to ping me next Tuesday if I haven't gotten back to you by then. |
…n logic Co-authored-by: Cursor <cursoragent@cursor.com>
…dcoding hasColorscale to true
… and calendar handling
…h standard marker.color
…move hard-coded ranges from mocks
…eGuide showLine check
….textPointStyle utility
…for unselected arrows
…leDefaults captures its reference
0b89f45 to
b8559e2
Compare
acfd7f0 to
1a15aa3
Compare
|
hey there, @emilykl, hope you are having a great start to the week! i just rebased and pushed again 🥳 |
|
@degzhaus would you be able to merge/rebase one more time? Sorry for the extra work. We just changed how our test images are generated and that led to the baseline conflicts with this branch. |
|
Hi @degzhaus ! I've run down the list of issues from my previous review, and most seem to be fixed 🥳 However I am still seeing these two issues:
The baseline images
The baseline images in I'm still doing a bit more testing so I will leave another comment if I find anything else. @camdecoster is taking a look as well, so keep an eye out for his comments. Finally when you have a chance please merge/rebase against the master branch! We recently overhauled the CI pipeline and you will need to merge/rebase in order to run the new pipeline. |
| }, | ||
| anchor: { | ||
| valType: 'enumerated', | ||
| values: ['tip', 'tail', 'cm', 'center', 'middle'], |
There was a problem hiding this comment.
Could you please remove 'cm' and 'center' as values? One term for the middle should be fine.
| }; | ||
|
|
||
| // Extend with base attributes (includes hoverinfo, etc.) | ||
| extendFlat(attrs, baseAttrs); |
There was a problem hiding this comment.
Did you mean to pasate baseAttrs on top of attrs? I think it would be safer to handle it this way. That's what is typically done when building attributes.
| extendFlat(attrs, baseAttrs); | |
| attrs = extendFlat({}, baseAttrs, attrs); |
| var trace = cd[0].trace; | ||
| var marker = trace.marker || {}; | ||
| var markerLine = marker.line || {}; | ||
| var lineColor = Lib.isArrayOrTypedArray(marker.color) ? undefined : marker.color; |
There was a problem hiding this comment.
If marker.color is an array, lineColor will be undefined. This would result in the unselected color being undefined as well. Could you update the code to handle this situation and apply a dim version (via opacity) of the selected color?
|
|
||
| // Colorscale cmin/cmax computation: prefer provided marker.color, else magnitude | ||
| if(trace._hasColorscale) { | ||
| var vals = hasMarkerColorArray ? [cMin, cMax] : [normMin, normMax]; |
There was a problem hiding this comment.
Could you add a guard (here or elsewhere) that handles if normMin and normMax are infinite? This is an edge case that would require all invalid data, but we should guard against it.
| var Lib = require('../../../src/lib'); | ||
| var d3Select = require('../../strict-d3').select; |
There was a problem hiding this comment.
Could you delete these unused imports?
Overview
This PR adds a new
quivertrace type to Plotly.js for visualizing 2D vector fields using arrows.Continuation of #7584
Features
x,ycoordinates with direction/magnitude fromu,vcomponentsscaled(normalized),absolute, orrawvector lengths viasizemode/sizereftail,tip, orcenter/cm/middlecarray)line.*attributesarrowsizehovertemplate, point selectionx,y,u,v, andcarraysAPI
Screenshots
Examples taken from plotly.com/python/quiver-plots
Gist with example code
Basic Quiver Plot
With Colorscale
Testing
Files Changed
New files (
src/traces/quiver/)index.js- Trace module definitionattributes.js- Attribute schemadefaults.js- Default value handlingcalc.js- Data calculationplot.js- SVG renderingstyle.js- Stylinghover.js- Hover behaviorselect_points.js- Selection supportevent_data.js- Event data formattingformat_labels.js- Label formattingModified
lib/index.js&lib/index-strict.js- Build integrationTests
test/jasmine/tests/quiver_test.js- Unit teststest/image/mocks/quiver_*.json- 9 mock filestest/image/baselines/quiver_*.png- Baseline images