fix: separate star chart x labels on short windows - #730
Merged
Conversation
The tick format was chosen from whether the first and last star fell in the same calendar year, which says nothing about how far apart the ticks are. A two-hour window printed one date five times, and a week straddling 31 December was pushed to month resolution it could not separate. Pick the format from the gap between ticks instead, and draw a single tick when they all land on one instant. Closes #725
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
The chart's x-axis tick format was chosen like this:
X_TICKSis fixed at 5 and the ticks are spread evenly across the window, sowhat decides whether two of them collide is the gap between them. A calendar
year comparison says nothing about that gap, and it fails in both directions:
short windows collapse under
%b %d, and a short window that happens tostraddle 31 December is pushed all the way to
%b %Y, which cannot separateticks a few days apart.
Measured against the current renderer, reading the x-tick row:
Aug 01five timesAug 01four times,Aug 02Aug 01, Aug 01, Aug 02, Aug 03, Aug 04Dec 2026three times,Jan 2027twiceDec 2026, Dec 2026, Jan 2027, Jan 2027, Feb 2027Jan 01five timesCloses #725
Changes
_x_tick_formatpicks the coarsest format that still separates adjacentticks, from the gap between them: month and year at a year or more, day below
that, dropping to hours and then seconds as the gap shrinks.
separate ticks that are the same instant, and
_scale_xalready stacks themon a single pixel, so five copies was the wrong answer rather than a
formatting problem.
The 7-days-across-New-Year case now reads
Dec 28, Dec 29, Dec 31, Jan 02, Jan 04, which is both distinct and more useful than the month labels it producedbefore.
Testing
Full gate green:
2806 passed.The three new tests were run against the pre-fix renderer first and all three
fail there, which is the point of them:
Two properties worth calling out, both checked rather than assumed:
(200 to 3000 days) select the same formats as before.
both the old and new renderer gives byte-identical SVGs, and both match what
is currently published on
assets. That matters because the workflow treatsan unchanged render as a no-op, so a format change would have pushed one
pointless refresh commit.
Type of Change
fix:)feat:)refactor:)docs:)ci:)chore:)Checklist
Closes #Ntype:*and onepriority:*labelphase:*label (or none when not roadmap work)feat/<slug>,fix/<slug>, etc.)mypy src/; the script is clean under it too)ruff check .)ruff format --check .)