-
Notifications
You must be signed in to change notification settings - Fork 612
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
fix: remove incorrect stack transform from charts with one linear Q-axis one non-linear Q-axis. #8871
Conversation
2ec1077
to
0de5218
Compare
@@ -104,15 +104,6 @@ function potentialStackedChannel( | |||
// if there is no explicit stacking, only apply stack if there is only one aggregate for x or y | |||
if (xAggregate !== yAggregate) { | |||
return xAggregate ? x : y; | |||
} else { |
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 logic was just wrong. Instead, we should rely on the orient logic below.
examples/compiled/bar_q_qpow.png
Outdated
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.
Are these two images supposed to be before and after? I don't see any changes
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 is a new spec so there are new images.
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.
@ZacharyBys If you see the 4th (generated commits), you can see that the actual code changes in the 3rd commit fixes the problem.
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.
Looks good. There is a failing test for bar_q_qpow.vl.json, though.
…xis one non-linear Q-axis. (vega#8871) Co-authored-by: GitHub Actions Bot <vega-actions-bot@users.noreply.github.com>
Remove incorrect stack transform from charts with one linear Q-axis one non-linear Q-axis.