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

Introduce cartesian axis breaks #4614

Merged
merged 24 commits into from Mar 12, 2020
Merged

Introduce cartesian axis breaks #4614

merged 24 commits into from Mar 12, 2020

Conversation

etpinard
Copy link
Contributor

@etpinard etpinard commented Mar 3, 2020

which presents a solution for removing weekends, holidays and non-work hours as requested in #1382 and lays the groundwork from "visually skipped" ranges as in the screenshot in #4210.


In brief, this PR adds a breaks array attribute to cartesian axes. Each item inside breaks defines an axis break either using a 2-item bounds array or a free-length array of values. When using bounds on date axes, one can set a pattern to generate breaks e.g. over all weekends or non-work hours. When using values, one can set the "spread" of each value using dvalue. The inclusive/exclusive behaviour at the bounds is determined by the operation attributes (as used in filter transforms).

Some examples:

To implement axis breaks,

  • we first set coordinates that fall within the breaks to BADNUM (more info in baf753a). Please note that this implies that lines remain continuous across a break if there's no (x,y) pt inside the break in question.
  • sanitize the breaks present inside the axis range during ax.setScale (i.e. when the range is known) and adapt the l2p and p2l mappings (more info in 259eafa)
  • Use the sum of the piecewise data-space ranges between the breaks (instead of simply range[1] - range[0]) for autorange padding, ticks placement computations and during drag interactions.

cc @archmoj @alexcjohnson @nicolaskruchten @antoinerg

... including potential future attributes `gap` and `gapmode`
- used during makeCalcdata to mask (i.e. set to BADNUM) coordinates
  inside the axis breaks.
- add abstractions:
  + ax._breaks, the disjoint breaks inside the range though ax.locateBreaks
  + ax._lBreaks, length of these breaks in value space
  + ax._m2, l2p slope (same for all intervals)
  + ax._B, set of l2p offsets (one for each of the (N+1) piecewise intervals)
- adapt l2p and p2l for axis breaks
- add jasmine tests for setConvert and hover labels
... as the value-space length is reduced when axis breaks are
    present within the trial range
- reduce the value-space range by `ax._lBreaks` to
  get the correct the number of auto ticks on the axes.
- add logic to remove ticks falling inside breaks
- add (could do better) logic for ticks that overlap
  on either side of a break.
- test breaks on x axes
- test breaks on y axes
- test date axes with patterns
- test breaks with tickvals and ticktext
- test for zeroline inside breaks
itemOut.enabled = false;
return;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

What about handling autorange: 'reversed' case?
In the following demos I tried to flip the axes which it didn't work.
https://codepen.io/MojtabaSamimi/pen/yLNzLra?editors=0010
https://codepen.io/MojtabaSamimi/pen/bGdoGyG?editors=0010
https://codepen.io/MojtabaSamimi/pen/QWbqWRj?editors=0010

Copy link
Contributor Author

@etpinard etpinard Mar 4, 2020

Choose a reason for hiding this comment

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

Ah right, I forgot to test those. They won't be handled here as this logic has to do with set (i.e. autorange: false) ranges, but yeah I'll get this fixed. Thanks!

break;
}

while(t <= r1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we add a finite test for t before entring this loop?
Is there any scenario (e.g. on reversed axes) in which step could be negative?

@nicolaskruchten
Copy link
Member

Looks great! Do we have any control over the visual aspect of the break? Can we put in a gap or a squiggle or something?

@etpinard
Copy link
Contributor Author

etpinard commented Mar 4, 2020

Do we have any control over the visual aspect of the break? Can we put in a gap or a squiggle or something?

Not at the moment.

/*
gap: {
valType: 'number',
min: 0,
dflt: 0, // for *date* axes, maybe something else for *linear*
editType: 'calc',
role: 'info',
description: [
'Sets the gap distance between the start and the end of this break.',
'Use with `gapmode` to set the unit of measurement.'
].join(' ')
},
gapmode: {
valType: 'enumerated',
values: ['pixels', 'fraction'],
dflt: 'pixels',
editType: 'calc',
role: 'info',
description: [
'Determines if the `gap` value corresponds to a pixel length',
'or a fraction of the plot area.'
].join(' ')
},
*/
// To complete https://github.com/plotly/plotly.js/issues/4210
// we additionally need `gap` and make this work on *linear*, and
// possibly all other cartesian axis types. We possibly would also need
// some style attributes controlling the zig-zag on the corresponding
// axis.

@nicolaskruchten
Copy link
Member

OK. I feel like introducing breaks without visible gaps is a bit scary in other contexts than "drop weekends for business metrics" ... what's the LOE on visible gaps?

@etpinard
Copy link
Contributor Author

etpinard commented Mar 4, 2020

what's the LOE on visible gaps?

Probably 1-2 days for gaps w/o squiggle on the axis. I suspect that getting everyone happy with the squiggle look/behaviour will take a few days on its own.

@nicolaskruchten
Copy link
Member

Zooming way in to the pattern date mock yields some odd tick labels:

image

- pass axis breaks to mock rangeslider figures
- consider axis breaks in rangeslider d2p & p2d
- add mocks + drag test
... that way, positions that include some sort of offset
    (e.g. bar x0/x1, ohlc xo/xc) have the adequate
    px position.
@nicolaskruchten
Copy link
Member

To capture what was discussed today: for 1.53 let's activate this feature for date axes only please. We'll need to think about axis gap and bar charts etc before turning this on on linear/log axes I think.

@etpinard
Copy link
Contributor Author

etpinard commented Mar 5, 2020

TODO:

  • coerce breaks only on type: 'date' axes, adapt tests and mocks!
  • make sure axis breaks work on axes with range[0] > range[1]

see comments over on: axis-breaks...axis-breaks-fixups

@archmoj will attempt to fix these tomorrow and on Monday the 9th.

  • better "first tick" algorithm e.g. what to do when the canonical first auto tick falls into a break? See b5aaf92
  • better "overlapping tick in either side of an axis break fix" algorithm, see comment in 6bec94d

I should be able to look at these when I get back Tuesday the 10th.

@etpinard
Copy link
Contributor Author

  • better "first tick" algorithm e.g. what to do when the canonical first auto tick falls into a break?

Commit 28c328d made things a bit better. Still room for improvement though.

Improved once more in 1b55b42


Pan fixes from @archmoj are in 40d57fa


Should we warn the user in that case?
Could we visible: false scattergl on date axes with breaks?

Yes, let's leave gl out of this PR. Hide them - and log a warning - in that case. But @archmoj please make an issue to come back to this soon, I can see this being important for some users.

Done in 49b4053

@archmoj
Copy link
Contributor

archmoj commented Mar 12, 2020

Great PR!
🎖️ 🎖️ 🥇

Thanks very much @etpinard for tackling all this.
💃 🕺

@etpinard
Copy link
Contributor Author

etpinard commented Mar 12, 2020

Ok, merging to get a first round of QA rolling (I suspect a few bug reports will come out of it).

Updated examples:

breaks using pattern on date axis: https://codepen.io/etpinard/pen/PoqRNoL
breaks using values on date axis: https://codepen.io/etpinard/pen/OJVvNJG

@etpinard etpinard merged commit 7ca1777 into master Mar 12, 2020
@etpinard etpinard deleted the axis-breaks branch March 12, 2020 21:23
@nicolaskruchten
Copy link
Member

🏆 thanks @etpinard 🙇

@kenahoo
Copy link

kenahoo commented Oct 1, 2020

Is this functionality part of any release? Or is it still experimental? I'm having a little trouble finding how to do it in the docs.

@archmoj
Copy link
Contributor

archmoj commented Oct 2, 2020

Is this functionality part of any release? Or is it still experimental? I'm having a little trouble finding how to do it in the docs.

@kenahoo yes it is available since v1.53.0
You could find javascript documentation at https://plotly.com/javascript/reference/layout/xaxis/#layout-xaxis-rangebreaks.
The documentation for Python at the bottom of this page https://plotly.com/python/time-series/ could also be of interest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants