Skip to content

center-align confusion matrix title#160

Merged
dberenbaum merged 2 commits into
treeverse:mainfrom
sisp:fix/confusion-matrix-title-center
Apr 10, 2024
Merged

center-align confusion matrix title#160
dberenbaum merged 2 commits into
treeverse:mainfrom
sisp:fix/confusion-matrix-title-center

Conversation

@sisp

@sisp sisp commented Apr 9, 2024

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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

sisp commented Apr 9, 2024

Copy link
Copy Markdown
Contributor Author

@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
Copy Markdown
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

sisp commented Apr 10, 2024

Copy link
Copy Markdown
Contributor Author

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:

@dberenbaum dberenbaum left a comment

Copy link
Copy Markdown
Contributor

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 treeverse:main Apr 10, 2024
@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