Skip to content
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

Add barpolar traces #2954

Merged
merged 32 commits into from
Sep 6, 2018
Merged

Add barpolar traces #2954

merged 32 commits into from
Sep 6, 2018

Conversation

etpinard
Copy link
Contributor

@etpinard etpinard commented Aug 29, 2018

resolves #2810 (not everything item on there, but enough to replace legacy polar's area trace type)

Say hello to Polar 2.0's version of 'area' traces: barpolar. As discussed previously, this trace type is "really" like a bar chart but in polar coordinates, hence its name. Here's a much-awaited updated version of our wind rose example:

peek 2018-08-29 15-50

Moreover, barpolar isn't just "really like" bar from a user's standpoint, but also from a dev's standpoint: I was able to reuse almost all of Bar.crossTraceCalc (formally known as Bar.setPositions), its default logic is a mix of ScatterPolar and Bar routines.

The hard part about this PR was to keep everything DRY. Many path-drawing routines in plots/polar.js had to be generalized in order to be used in BarPolar.plot. See commits 4d20388, 95119c4 and facb2f8 for more details.

In brief, this first version of barpolar, includes support for:

  • barmode stacked and overlay
  • bar width, offset and base
  • per-subplot bargap
  • hovermode: 'closest', selections

But, does not implement:

  • orientation 'angular'
  • on-graph text

cc @alexcjohnson @antoinerg

- as now _module.plot doesn't clear the <g.trace> layers
  on every call, we don't need to call _module.style
  to render the correct styling
... which is more precise and less confusing (I think)
- add move findIndexOfMin to lib/search.js
... their polygon-equivalent to helpers.js

  Then, add wrappers on Polar.prototype, to handle is
  `gridshape: 'linear'` cases.
includes support for:
- barmode stacked and overlay
- bar width, offset and base
- per-subplot bargap

does not support:
- orientation 'angular'
- on-graph text
- w/ reversed radial range
- radial category
- radial date
- radial log
@etpinard etpinard added bug something broken feature something new status: reviewable labels Aug 29, 2018
@etpinard etpinard added this to the v1.41.0 milestone Aug 29, 2018
src/traces/barpolar/plot.js Outdated Show resolved Hide resolved
src/lib/angles.js Outdated Show resolved Hide resolved
src/lib/index.js Outdated Show resolved Hide resolved
src/lib/angles.js Outdated Show resolved Hide resolved
@alexcjohnson
Copy link
Collaborator

Fantastic job @etpinard - look and feel is beautiful, you've got more than enough here for a v1. Just a few comments above and we'll be ready to go!

"theta": [0, 45],
"opacity": 0.7,
"marker": {
"line": {"width": 5}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which looks like

image

closeup

image

with the default stroke-mitelimit.

- so that bar and barpolar can DRY up their plot logic
- need to rename histogram "hover positions" p0/p1 -> ph0/ph1
if(!cumulativeSpec.enabled) {
cdi.pts = inputPoints[i];
if(uniqueValsPerBin) {
cdi.p0 = cdi.p1 = (inputPoints[i].length) ? pos0[inputPoints[i][0]] : pos[i];
cdi.ph0 = cdi.ph1 = (inputPoints[i].length) ? pos0[inputPoints[i][0]] : pos[i];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping @alexcjohnson I hope you don't mind.

The bar corners p0/p1/s0/s1 are now set during Bar.crossTraceCalc (moved from Bar.plot) so that we don't have to duplicate logic in BarPolar.plot. In turn, we needed to rename histogram p0/p1 to not break histogram hover.

src/lib/mod.js Outdated Show resolved Hide resolved
src/lib/angles.js Outdated Show resolved Hide resolved
@alexcjohnson
Copy link
Collaborator

Very nice 💃

@etpinard etpinard merged commit 33e1acf into master Sep 6, 2018
@etpinard etpinard deleted the barpolar-traces branch September 6, 2018 00:44
@etpinard etpinard mentioned this pull request Sep 6, 2018
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken feature something new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Polar 2.0 replacement for legacy "area" type
2 participants