Time scale now compensates for rounded tick units #2371
Conversation
|
Fix looks ok. Should hold off on merging until #2346 |
|
Sounds good |
| this.tickUnit = unitDefinition.name; | ||
| this.scaleSizeInUnits = this.lastTick.diff(this.firstTick, this.tickUnit, true); | ||
| this.leadingUnitBuffer = this.firstTick.diff(this.firstTick.clone().startOf(this.tickUnit), this.tickUnit, true); | ||
| this.scaleSizeInUnits = this.lastTick.diff(this.firstTick, this.tickUnit, true) + (this.leadingUnitBuffer > 0 ? 2 : 0); |
There was a problem hiding this comment.
is 2 always the right number in this case? I was thinking about this more and I think it could introduce the possibility of having 1 to many ticks in certain cases
There was a problem hiding this comment.
I'm just going off of what worked in the wild. I tried various data points that left rounding space at the beginning and end. Have you tried any thing with it that you think would break?
There was a problem hiding this comment.
I think the time it would break was if the first or last data point was exactly at the start of the new unit already.
If the firstTick was 12AM, then rounding to 'day' wouldn't cause a change
There was a problem hiding this comment.
You're right. But the buffer should only gets used if the difference is greater than zero right?
There was a problem hiding this comment.
For the first tick, yes. But if the last tick was at the end of the day and the unit became 'day' we'd have the same thing. I think the check might need to be expanded, but I'm not 100% sure
Closes #2277