Skip to content
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

Auto mark: zero baseline default for rules #1341

Merged
merged 2 commits into from
Mar 15, 2023
Merged

Auto mark: zero baseline default for rules #1341

merged 2 commits into from
Mar 15, 2023

Conversation

tophtucker
Copy link
Contributor

@tophtucker tophtucker commented Mar 15, 2023

Mike writes:

The mark-based zero check is also forgetting about rule. This should have a y = 0 rule like rect does.

Now it does!

Before After
autoRuleZero-changed autoRuleZero
Plot.auto(olympians, {x: "date_of_birth", y: {value: "height", reduce: "mean"}, mark: "rule"}).plot()

A couple notes:

barY or areaY or rectY or… ruleX? Yes; ruleY would be the one parallel to the y baseline, and can make sense without any zero baseline; ruleX is the one orthogonal to the baseline, which has to “stand on it” (like barY).

Merely adding || mark === ruleY and || mark === ruleX to the conditional breaks autoNullReduceContinuous and autoNullReduceDate, so I also added the X && and Y && checks, which seem like a good idea anyway.

Broken Fixed
autoNullReduceContinuous-changed autoNullReduceContinuous

The new test case demonstrates the existing issue that a rule can overhang its baseline… but that sounds like a whole can of worms and I wouldn't worry about it lol:

image

Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. 👍

@mbostock mbostock merged commit 93f953a into main Mar 15, 2023
@mbostock mbostock deleted the toph/zero-rule branch March 15, 2023 16:01
chaichontat pushed a commit to chaichontat/plot that referenced this pull request Jan 14, 2024
* Auto mark: zero baseline default for rules

* fix existing tests, commit new test output
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants