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

Matplotlib table: use bold weight when calculating header width #90

Merged

Conversation

padgy
Copy link
Contributor

@padgy padgy commented May 7, 2023

When using Matplotlib as converter I was getting overlapping text for long column headers. This appears to be because the headers are bold but we do not set the weight as bold when calculating headers.

These changes fixed the issue for me.

@padgy padgy changed the title Matplotlib table: use bold for header width Matplotlib table: use bold when calculating header width May 7, 2023
@padgy padgy changed the title Matplotlib table: use bold when calculating header width Matplotlib table: use bold weight when calculating header width May 7, 2023
@PaleNeutron
Copy link
Collaborator

Can you add long bold header to case test_styled in tests/test_df_image.py to make sure this feature works fine afterwards?

@padgy
Copy link
Contributor Author

padgy commented May 8, 2023

Can you add long bold header to case test_styled in tests/test_df_image.py to make sure this feature works fine afterwards?

@PaleNeutron I don't think adding a bold header style is necessary to test this. The matplotlib converter produces bold headers already regardless of the style used:

I can add a long column header to the covid19.csv test data though if that is what you mean.

@PaleNeutron
Copy link
Collaborator

PaleNeutron commented May 9, 2023

Can you add long bold header to case test_styled in tests/test_df_image.py to make sure this feature works fine afterwards?

@PaleNeutron I don't think adding a bold header style is necessary to test this. The matplotlib converter produces bold headers already regardless of the style used:

I can add a long column header to the covid19.csv test data though if that is what you mean.

I mean some corner cases, for example, dataframe's index is th and what happens to it if it is very long?

And if user make one cell text bold by styler and the text is quite long, will it cause overlap?

But if you do not want to implement this is OK too. I can merge this after you add long header test case and a TODO at the place.

@padgy
Copy link
Contributor Author

padgy commented May 11, 2023

Can you add long bold header to case test_styled in tests/test_df_image.py to make sure this feature works fine afterwards?

@PaleNeutron I don't think adding a bold header style is necessary to test this. The matplotlib converter produces bold headers already regardless of the style used:

I can add a long column header to the covid19.csv test data though if that is what you mean.

I mean some corner cases, for example, dataframe's index is th and what happens to it if it is very long?

And if user make one cell text bold by styler and the text is quite long, will it cause overlap?

But if you do not want to implement this is OK too. I can merge this after you add long header test case and a TODO at the place.

No problem! Just added a test for a dataframe with long column headers. If you run the test and check the images in the test output folder you should see the headers do not overlap.

As for edge cases, like making one cell bold etc, correct me if I am wrong, but it seems that bold styles applied to the body do not work in the current implementation? Likely due to the following:

https://github.com/dexplo/dataframe_image/blob/eff1f4dcc2256c704adefff8f04e01afe5d4e2de/dataframe_image/_matplotlib_table.py#L102C5-L112

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