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

center-align confusion matrix title #160

Merged
merged 2 commits into from
Apr 10, 2024
Merged

center-align confusion matrix title #160

merged 2 commits into from
Apr 10, 2024

Conversation

sisp
Copy link
Contributor

@sisp sisp commented Apr 9, 2024

I've fixed the alignment of the confusion matrix title to be centered. Before, it was left-aligned which wasn't consistent with other plots – and IMO doesn't look nice.

Before After
before after

@dberenbaum
Copy link
Contributor

Thanks @sisp! What do you think of adding the same to all the plots so we ensure it stays consistent regardless of vega-lite updates?

@sisp
Copy link
Contributor Author

sisp commented Apr 9, 2024

@dberenbaum We could probably do that. Do you happen to have a demo of all Vega templates to check that "anchor": "middle" renders correctly for all of them?

@dberenbaum
Copy link
Contributor

I think the closest we have is https://github.com/iterative/vscode-dvc-demo/blob/main/dvc.yaml. It should be easy to adjust the templates there to check the others (for example, bar_horizontal -> bar_horizontal_sorted).

@sisp
Copy link
Contributor Author

sisp commented Apr 10, 2024

I've added the anchor: middle setting to all Vega-Lite plot titles. The demo plots except confusion matrix are unchanged. See the attached reports before and after the change:

Copy link
Contributor

@dberenbaum dberenbaum left a comment

Choose a reason for hiding this comment

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

Thanks @sisp!

@dberenbaum dberenbaum merged commit 5671fe3 into iterative:main Apr 10, 2024
14 checks passed
@sisp sisp deleted the fix/confusion-matrix-title-center branch April 10, 2024 14:44
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.

2 participants