-
Notifications
You must be signed in to change notification settings - Fork 182
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
better ordinal axes with intervals #1790
Conversation
25f2a52
to
2afe52a
Compare
A nice property would be if e.g. “2018” always meant the same thing in the default time axis, i.e. 2018-01-01. So perhaps a better strategy here would be:
The nice thing about this strategy is that it avoids needing to know exactly what the scale’s actual interval is; the scale’s interval can be a black box, and we can infer everything from applying the interval and looking at domain values. |
2afe52a
to
bc7ec6e
Compare
Got some pieces working here: https://observablehq.com/d/61f1586d16d3ec0c |
11d50fd
to
054ed6e
Compare
Now also addresses part of #74 when a numeric interval is specified. |
Okay! Ready to go! 🚀 |
Okay! The latest commit detects and generalizes standard time intervals, so we get the best possible result for the common case where the scale’s interval is something common like month or day. Plot.plot({
x: {interval: "month"},
marks: [Plot.barY(aapl, Plot.groupX({y: "median", title: "min"}, {title: "Date", x: "Date", y: "Close"}))]
}) I believe I’m done polishing this now. I feel like even though this was a long journey, the resulting logic isn’t too complicated. Let me know if I can clarify anything, @Fil. |
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.
this chart is a bit surprising now; maybe we'd need to replace the y grid by a ruleY?
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.
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.
Something like…
: tickFormat === undefined && data && isNumeric(data) && scale.interval?.[intervalDuration]
? format(`.${`${scale.interval?.[intervalDuration] % 1}`.length - 2}f`)
but hopefully not so messy. 😁
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.
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'd add more ticks in this case, but it's nice to show what it gets automatically.
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.
so much better!
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.
this test loses an empty text…
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.
… and this test gains an empty text; not a blocker at all, but out of curiosity what's the logic here?
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 noticed that too but didn’t investigate. 🤷
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.
nice!!
None of the above comments is blocking. I poked at the downloadsOrdinal example a bit and maliciously tried (Note that x: {interval: "second"} takes a dozen of seconds but ends up returning a chart — except of course that the bars are too thin to be visible.) |
Using temporal intervals & tickSpacing on the horizontal scale is really flowing. I'm adding a test that uses it on the fy facet scale. |
90f71ce
to
701a8b7
Compare
* ordinal time axis * filter ordinal ticks with numeric intervals * checkpoint * simplify hasTimeTicks * fix nullish check * filter approach * inferTimeFormat * tidy * prune redundant formats * tidy * comment * filter ticks, not just text * warn on misaligned intervals * dense grid for sparseCell * add missing test snapshot * more robust inferTimeFormat * detect and generalize standard time intervals * test: temporal interval on the facet scale * improve temporal scales, too * better edge cases * tweak comment * move tickFormat function detection * minimize diff --------- Co-authored-by: Philippe Rivière <[email protected]>
This applies the nice multi-line time axis to ordinal scales that have time intervals. For example:
Still to-do:
I’m excited about this usability improvements since people often want to represent time ordinally.
Fixes #1789.