-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Period positioning for Cartesian traces #5074
Conversation
These will be needed in the near future for sure but if it makes sense to do them in a separate PR I'm fine with that for now :) The main use-case is for weekly data i.e. |
Could we please add a mock and CodePen that has the following settings?
|
|
Excellent, thank you :) |
|
The codepen in the PR description (as well as other mocks) cover this case. |
Whew, that's a lot of trace types! Great image tests, then it seems to me we need a hover test with a very detailed |
Thanks! |
…tick and histograms - and do not support violin for now
src/plots/cartesian/align_period.js
Outdated
var d = new Date(dateStr); | ||
var year = d.getUTCFullYear(); | ||
var month = d.getUTCMonth(); | ||
var day = d.getUTCDate(); |
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.
I think this approach is still going to have problems with time zones - specifically, I think the day will be wrong this way if you're in the eastern hemisphere.
There would be ways around this, but we've already solved these problems and all sorts of other edge cases with functions in lib/dates
so you never directly need Date
objects again and all your processing is with millisecond numbers. What I was thinking of in #5074 (comment) was something like:
var v = vals[i];
// guess at how many periods away from base we are
var nEstimated = Math.round((v - base) / (mPeriod * ONEAVGMONTH));
var endTime = incrementMonth(base, mPeriod * nEstimated, calendar);
// iterate to get the exact bounds before and after v
// there may be ways to make this faster, but most of the time
// you'll only execute each loop zero or one time.
while (endTime > v) {
endTime = incrementMonth(endTime, -mPeriod, calendar);
}
while (endTime <= v) {
endTime = incrementMonth(endTime, mPeriod, calendar);
}
// now we know endTime is the boundary immediately after v
// so startTime is obtained by incrementing backward one period.
var startTime = incrementMonth(endTime, -mPeriod, calendar);
newVals[i] = (
isStart ? startTime :
isEnd ? endTime :
(startTime + endTime) / 2;
)
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.
Thanks for your review.
Revised in 83b22a0.
- and improve test for the case of stack and default weekly period on Monday
|
||
function getPeriod0Dflt(period, calendar) { | ||
var n = period / ONEWEEK; | ||
return dateTick0(calendar, Math.round(n) === n); |
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.
Not blocking, but you can also do (period / ONEWEEK) % 1 === 0
Something funny in period_positioning2 |
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.
Looks great. Just a minor question about some missing labels #5074 (comment), and I'd be happy to come back to that later if it's not an easy fix. 💃
Resolves #4912 | Demo
Implementing
(x|y)period
,(x|y)period0
,(x|y)periodalignment
ondate
axes for Cartesian traces namely:scatter
,scattergl
,bar
,funnel
,waterfall
,ohlc
,candlestick
,box
,contour
,heatmap
.i.e. to position marks along date axes not at a particular instant in time, but at the start/midpoint/end of a well-defined period.
New trace-level attributes:
(x|y)period: Standard duration notation like ‘m’ or millis
(x|y)period0: A specific boundary point timestamp which is used to compute all boundaries.
(x|y)periodalignment: start/middle/end
update:
Please note that this feature is unimplemented for
histogram*
traces by #5175; whereas one could simply use bins.@plotly/plotly_js