Fix overlapping auto-generated ticks #6243
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It was reported in #6109 that the time scale sometimes generates overlapping ticks. @kurkle debugged this and determined in #6115 that the issue is because the time scale's auto-tick generation doesn't consider padding.
We'd like to take most of the intelligence out of the time scale's auto tick generation and instead generate a tick at every point and use the auto-skipper to automatically decide which ticks to display. For more details see #4612
While the solution proposed in #6115 fixes the bug it also adds more logic to the time scale which is not quite going in the direction we'd like to take things longer-term. This PR may seem a bit funny standalone because when
ticks.source: 'auto'
we're auto-generating ticks and then checking to see if we have too many and autoskipping some of them as well. We shouldn't really have two auto-tick logics. I have a branch on my machine locally that moves much of the time scale's auto-generation logic to the auto-skipper so that everything is contained there. The change in this PR is both necessary for that longer term solution and also in the short-term fixes the bug reported in #6109Closes #6109 #6115