[WIP] Carpet plots #1239
[WIP] Carpet plots #1239
Conversation
|
Interesting question, @etpinard @alexcjohnson: If we allow scatter plots and inequality contours, I'm not sure, generally speaking that we can guarantee points will lie on the correct side of the inequality. That is, bicubic interpolation of the data value at every point along the contour won't actually equal the contour value. Splines handle affine transforms nicely, but if the grid lines themselves are cubic splines, then the true contour lines that cut across those grid lines subject to bicubic interpolation would be more complicated than cubic splines themselves and could probably only be drawn correctly by oversampling. I'm glad to clarify that if needed, but just something I'm realizing as I go through this. That was part of the initial motivation for a high resolution grid. We could subdivide to reduce the problem, but I'm not yet sure what the best option will be. Or this could just not be a concern since you shouldn't really be relying upon that to make any judgments in the first place. (NURBS might be sufficient, but I think you just have to solve it numerically anyway and that would still just have to be drawn by subdividing) |
|
When you say "the true contour" are you referring to the exact path the contour takes within one grid cell being different if you draw it in a,b space and map back to x,y vs if you drew it in x,y space in the first place? I'm actually not sure if there is an objectively true contour in that sense... in fact if you've chosen a really good carpet transformation, the contours you draw in x,y space might even be more meaningful at sub-cell resolution than in a,b space because the variation of physically relevant properties might be more homogeneous than in a,b space (where they could be stretched along a diagonal for example). Also what do you mean by "high-resolution grid" - isn't the grid just whatever data we get from the user? |
|
Conclusion after talking with @alexcjohnson: The actual real-world implications of this and the cost of doing better (i.e. numerical spline-fitting) make it negligible for now as long as it's documented as such. |
rreusser
force-pushed
the
carpet-plots
branch
from
7164f68 to
5aa52ce
Compare
src/lib/carpet.js
Outdated
|
|
||
| 'use strict'; | ||
|
|
||
| module.exports = require('../src/traces/carpet'); |
There was a problem hiding this comment.
Hmm. This file doesn't do anything, right?
There was a problem hiding this comment.
Maybe it shouldn't be included by default, but see:
Line 41 in 3f18627
There was a problem hiding this comment.
Right lib/carpet.js is required, but src/lib/carpet.js isn't.
| visible: { | ||
| valType: 'boolean', | ||
| role: 'info', | ||
| description: [ |
There was a problem hiding this comment.
Could reuse the declaration from cartesian/ with visible: axesAttrs.visible,
There was a problem hiding this comment.
Good point. I blindly copied this definition in a rush since it broke gl3d plots entirely until it was present.
src/plots/plots.js
Outdated
| // there's anything to sort: | ||
| hasPriorities = hasPriorities || priority; | ||
|
|
||
| prioritizedFullDataLookup.push({index: i, priority: priority}); |
There was a problem hiding this comment.
Oh, we need this after all? I thought we concluded it wasn't necessary at some point in #plotly_js. Honesty, it doesn't add much code, so I'm 👍 either way.
There was a problem hiding this comment.
That's a good question… I thought the same, but there it is. I'll check on the removability.
There was a problem hiding this comment.
Reverting this. I reordered one of the mocks to ensure that data is in an order that would cause it to fail if this were required but not implemented.
src/traces/carpet/attributes.js
Outdated
| var colorAttrs = require('../../components/color/attributes'); | ||
|
|
||
| module.exports = { | ||
| carpetid: { |
There was a problem hiding this comment.
id isn't the most plotly-esque of identifiers. It's probably the best for this job though. But perhaps, carpetname or simply carpet would sound more familiar to plotly users?
There was a problem hiding this comment.
carpet works for me. Should an easy change.
src/traces/carpet/axis_attributes.js
Outdated
| valType: 'number', | ||
| dflt: 1, | ||
| min: 0, | ||
| max: 1, |
There was a problem hiding this comment.
it would be nice to make this consistent with the scatter definition of smoothing where the default value is 1 and the max 1.3.
src/traces/contourcarpet/plot.js
Outdated
| }); | ||
| } | ||
|
|
||
| /* function clipGaps(plotGroup, plotinfo, cd0, perimeter) { |
There was a problem hiding this comment.
Yup. TODO. I'll remove it. It's in the git history or just copied from contour anyway.
src/traces/contourcarpet/style.js
Outdated
| .call(Drawing.lineGroupStyle, | ||
| line.width, | ||
| line.color, | ||
| // contours.coloring === 'lines' ? colorMap(start + i * cs) : line.color, |
src/traces/scattercarpet/index.js
Outdated
|
|
||
| ScatterCarpet.attributes = require('./attributes'); | ||
| ScatterCarpet.supplyDefaults = require('./defaults'); | ||
| // ScatterCarpet.colorbar = require('../scatter/colorbar'); |
There was a problem hiding this comment.
I guess should just work as scattercarpet is using the same colorscale attribute as the cartesian version.
There was a problem hiding this comment.
Correct. Uncommented. ✅
| var scatterSelect = require('../scatter/select'); | ||
|
|
||
|
|
||
| module.exports = function selectPoints(searchInfo, polygon) { |
There was a problem hiding this comment.
Hey, why not? Does this work?
| var scatterStyle = require('../scatter/style'); | ||
|
|
||
|
|
||
| module.exports = function style(gd) { |
There was a problem hiding this comment.
This module isn't registered in scattercarpet/index.js. TODO or 🔪 ?
There was a problem hiding this comment.
I've included it and restored the same check that scatterternary uses to either run it or take for granted that the scatter trace has already run it.
|
Ah, I see. My confusion. 👍 (Edit: oh yeah… that's the main entry point… removing it makes everything fail… gotcha) |
|
Superseded by #1595. |
Trace types to be implemented:
carpettickmode: 'array'axesscattercarpetcontourcarpetCurrent state:
Live version: https://rreusser.github.io/demos/plotly-unsupported/carpet.html